Bug 186822 – [Fullscreen] Page sometimes ends up with an incorrect zoom level after entering fullscreen

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Jer Noble]

[Fullscreen] Page sometimes ends up with an incorrect zoom level after entering fullscreen

[Jer Noble]

Created attachment 343106 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 343106 [details]
Patch

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

> Source/WebCore/dom/Document.h:395
> +    WEBCORE_EXPORT void setShouldOverrideViewportArguments(const std::optional<ViewportArguments>&);

A "set should override" method would take a bool, not an optional arguments. This should be setOverrideViewportArguments.

> Source/WebKit/UIProcess/WebPageProxy.h:563
> +    void setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>&);

This name is also wrong.

> Source/WebKit/WebProcess/WebPage/WebPage.h:894
> +    void setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>&);

Naming.

> Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2485
> +void WebPage::setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>& arguments)
> +{
> +    if (auto* document = m_page->mainFrame().document())
> +        document->setShouldOverrideViewportArguments(arguments);
> +}

This is scary. What happens if frames come and go while you're in fullscreen?

[EWS Watchlist]

Comment on attachment 343106 [details]
Patch

Attachment 343106 [details] did not pass win-ews (win):
Output: https://webkit-queues.webkit.org/results/8274044

New failing tests:
http/tests/security/canvas-remote-read-remote-video-localhost.html

[EWS Watchlist]

Created attachment 343219 [details]
Archive of layout-test-results from ews206 for win-future

The attached test failures were seen while running run-webkit-tests on the win-ews.
Bot: ews206  Port: win-future  Platform: CYGWIN_NT-6.1-2.9.0-0.318-5-3-x86_64-64bit

[Jer Noble]

(In reply to Simon Fraser (smfr) from comment #2)
> Comment on attachment 343106 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=343106&action=review
> 
> > Source/WebCore/dom/Document.h:395
> > +    WEBCORE_EXPORT void setShouldOverrideViewportArguments(const std::optional<ViewportArguments>&);
> 
> A "set should override" method would take a bool, not an optional arguments.
> This should be setOverrideViewportArguments.

Changed, here and elsewhere.

> > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2485
> > +void WebPage::setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>& arguments)
> > +{
> > +    if (auto* document = m_page->mainFrame().document())
> > +        document->setShouldOverrideViewportArguments(arguments);
> > +}
> 
> This is scary. What happens if frames come and go while you're in fullscreen?

Frames can't come and go in fullscreen; any navigation causes fullscreen to exit.

[Jer Noble]

Created attachment 343244 [details]
Patch

[Simon Fraser (smfr)]

> > > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2485
> > > +void WebPage::setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>& arguments)
> > > +{
> > > +    if (auto* document = m_page->mainFrame().document())
> > > +        document->setShouldOverrideViewportArguments(arguments);
> > > +}
> > 
> > This is scary. What happens if frames come and go while you're in fullscreen?
> 
> Frames can't come and go in fullscreen; any navigation causes fullscreen to
> exit.

Really? The document can add a frame outside of the fullscreen ancestor chain, right?

[Jer Noble]

(In reply to Simon Fraser (smfr) from comment #7)
> > > > Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:2485
> > > > +void WebPage::setShouldOverrideViewportArguments(const std::optional<WebCore::ViewportArguments>& arguments)
> > > > +{
> > > > +    if (auto* document = m_page->mainFrame().document())
> > > > +        document->setShouldOverrideViewportArguments(arguments);
> > > > +}
> > > 
> > > This is scary. What happens if frames come and go while you're in fullscreen?
> > 
> > Frames can't come and go in fullscreen; any navigation causes fullscreen to
> > exit.
> 
> Really? The document can add a frame outside of the fullscreen ancestor
> chain, right?

Yes, but it's only the main frame that matters.  And any navigation at any level will cause fullscreen to exit.

[WebKit Commit Bot]

Comment on attachment 343244 [details]
Patch

Clearing flags on attachment: 343244

Committed r233059: <https://trac.webkit.org/changeset/233059>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/41343831>