[webkit-reviews] review granted: [Bug 238483] [Cocoa] Automatically relayout the page when new fonts are installed : [Attachment 455985] Patch

[bugzilla-daemon at webkit.org]

Chris Dumez <cdumez at apple.com> has granted Myles C. Maxfield
<mmaxfield at apple.com>'s request for review:
Bug 238483: [Cocoa] Automatically relayout the page when new fonts are
installed
https://bugs.webkit.org/show_bug.cgi?id=238483

Attachment 455985: Patch

https://bugs.webkit.org/attachment.cgi?id=455985&action=review




--- Comment #4 from Chris Dumez <cdumez at apple.com> ---
Comment on attachment 455985
  --> https://bugs.webkit.org/attachment.cgi?id=455985
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=455985&action=review

r=me

> Source/WebCore/page/Page.cpp:392
> +    {

nit: Not convinced we need this scope.

> Source/WebCore/page/Page.cpp:396
> +		   forEachPage([](Page& page) {

nit: Could use auto&

> LayoutTests/fast/text/install-font-style-recalc.html:4
> +<script src="../../resources/js-test-pre.js"></script>

Can we just include js-test.js..

> LayoutTests/fast/text/install-font-style-recalc.html:20
> +<script src="../../resources/js-test-post.js"></script>

.. and not include the -post.js version?