[Webkit-unassigned] [Bug 197086] Layout of YouTube videos is wrong in fullscreen after the first time

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=197086

Darin Adler <darin at apple.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |darin at apple.com

--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 367784
  --> https://bugs.webkit.org/attachment.cgi?id=367784
Fix compilation error and ensure the titlebar shows in fullscreen

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

Have you tested this on the older versions of macOS that we need to support? Presumably the WebCoreFullScreenWindow changes affect Legacy WebKit as well; that’s why the class is in WebCore, not WebKit. How did you test Legacy WebKit?

> Source/WebCore/ChangeLog:11
> +        which ends up breaking the layout. Having that window use a style that incldues a

typo "incldues"

> Source/WebKit/ChangeLog:11
> +        which ends up breaking the layout. Having that window use a style that incldues a

typo "incldues"

> Source/WebKit/ChangeLog:15
> +        Declare a secret SPI to be used in WKFullScreenWindowController.

"an SPI", not "a secret SPI"

> Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm:2613
> +    return [[[WebCoreFullScreenWindow alloc] initWithContentRect:[[NSScreen mainScreen] frame] styleMask:(NSWindowStyleMaskTitled | NSWindowStyleMaskUnifiedTitleAndToolbar | NSWindowStyleMaskFullSizeContentView | NSWindowStyleMaskResizable) backing:NSBackingStoreBuffered defer:NO] autorelease];

Not new, but it’s messy to have this tricky technique split across multiple files. Would be nice if more of this could be in WKFullScreenWindowController. Maybe we should add a method +[WKFulScreenWindowController createWindow] that contains this code.

I’d like to see a comment explaining why we don’t use a borderless window so future programmers don’t need to look back at change log history to figure out why.

> Source/WebKit/UIProcess/mac/WKFullScreenWindowController.mm:130
> +    window.titlebarAlphaValue = 0;

I think this needs a why comment. Not clear what we are doing with titlebar alpha here.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20190421/f5ef8cf9/attachment.html>