[webkit-reviews] review granted: [Bug 171914] Add heuristic to avoid flattening "fullscreen" iframes : [Attachment 310112] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Jun 16 13:29:06 PDT 2017


Simon Fraser (smfr) <simon.fraser at apple.com> has granted Frédéric Wang (:fredw)
<fred.wang at free.fr>'s request for review:
Bug 171914: Add heuristic to avoid flattening "fullscreen" iframes
https://bugs.webkit.org/show_bug.cgi?id=171914

Attachment 310112: Patch

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




--- Comment #20 from Simon Fraser (smfr) <simon.fraser at apple.com> ---
Comment on attachment 310112
  --> https://bugs.webkit.org/attachment.cgi?id=310112
Patch

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

> Source/WebCore/page/Settings.h:93
> +    FrameFlatteningPartiallyEnabled,

Can we qualify "partially"? What kind of frames still get flattened?

> Source/WebCore/rendering/RenderIFrame.cpp:89
> +	       if (style().hasOutOfFlowPosition() &&
style().hasViewportUnits())
> +		   return false;

Maybe move this code into a separate function, since I imagine these heuristics
will evolve. The name of the function should reflect the enum value that is
currently "FrameFlatteningPartiallyEnabled".

> Source/WebKit/mac/WebView/WebPreferencesPrivate.h:163
> +- (unsigned)frameFlattening;
> +- (void)setFrameFlattening:(unsigned)flag;

unsigned -> WebKitFrameFlattening.

Actually I think you need to leave the existing functions alone, for API
compatibility, but still add the new ones.

> Tools/DumpRenderTree/mac/DumpRenderTree.mm:906
> +    [preferences setFrameFlattening:NO];

This should use the enum, not a BOOL.


More information about the webkit-reviews mailing list