Bug 192377 – We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag.

• Component: Layout and Rendering
• Status: RESOLVED FIXED

[Yongjun Zhang]

If the page specifies width=device-width in the viewport meta tag, we should use the native device width and ignore the minimum effective device width.

[Yongjun Zhang]

rdar://problem/46364206

[Yongjun Zhang]

Created attachment 356548 [details]
Patch.

[Tim Horton]

Comment on attachment 356548 [details]
Patch.

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

> Source/WebCore/page/ViewportConfiguration.cpp:151
> +void ViewportConfiguration::overrideDefaultConfiguration()

This is more of an "update" method than an actual "override" method, right? It doesn't always "override", it decides whether it should or not.

> Source/WebCore/page/ViewportConfiguration.cpp:161
> +    // When ignoring mEDW, we should render the content with native device width and use nativeWebpageParameters.

Please don't abbreviate like that :) It makes find-and-replace terrible! Also this is sort of a what and not a why comment. Might just leave it out

[Yongjun Zhang]

(In reply to Tim Horton from comment #3)
> Comment on attachment 356548 [details]
> Patch.
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=356548&action=review
> 
> > Source/WebCore/page/ViewportConfiguration.cpp:151
> > +void ViewportConfiguration::overrideDefaultConfiguration()
> 
> This is more of an "update" method than an actual "override" method, right?
Right, it is more like "update". I will change to that.
> It doesn't always "override", it decides whether it should or not.
> 
> > Source/WebCore/page/ViewportConfiguration.cpp:161
> > +    // When ignoring mEDW, we should render the content with native device width and use nativeWebpageParameters.
> 
> Please don't abbreviate like that :) It makes find-and-replace terrible!
> Also this is sort of a what and not a why comment. Might just leave it out
Sure, will do!

[Yongjun Zhang]

Created attachment 356613 [details]
Address review comments.

[Tim Horton]

Comment on attachment 356613 [details]
Address review comments.

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

> Source/WebCore/page/ViewportConfiguration.cpp:351
> +ViewportConfiguration::Parameters ViewportConfiguration::standardWebpageParameters()

We talked about the name of this offline. You might also consider sharing some code with the function above.

[Yongjun Zhang]

Created attachment 356762 [details]
Address review comments before landing.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 356762 [details]:

imported/w3c/web-platform-tests/WebCryptoAPI/generateKey/failures_AES-GCM.https.any.html bug 192050
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

The commit-queue encountered the following flaky tests while processing attachment 356762 [details]:

imported/w3c/web-platform-tests/css/geometry/interfaces.worker.html bug 192484 (author: [e-mail])
inspector/dom/customElementState.html bug 192485 (authors: [e-mail] and [e-mail])
The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 356762 [details]
Address review comments before landing.

Clearing flags on attachment: 356762

Committed r238946: <https://trac.webkit.org/changeset/238946>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.