Summary: | We should ignore minimumEffectiveDeviceWidth if the page specifies device-width in viewport meta-tag. | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Yongjun Zhang <yongjun_zhang> | ||||||||
Component: | Layout and Rendering | Assignee: | Yongjun Zhang <yongjun_zhang> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | bfulgham, commit-queue, simon.fraser, thorton, wenson_hsieh, yongjun_zhang, zalan | ||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||
Version: | WebKit Nightly Build | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Yongjun Zhang
2018-12-04 14:01:48 PST
Created attachment 356548 [details]
Patch.
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 (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! Created attachment 356613 [details]
Address review comments.
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. Created attachment 356762 [details]
Address review comments before landing.
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. 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: rego@igalia.com) inspector/dom/customElementState.html bug 192485 (authors: drousso@apple.com and rniwa@webkit.org) The commit-queue is continuing to process your patch. Comment on attachment 356762 [details] Address review comments before landing. Clearing flags on attachment: 356762 Committed r238946: <https://trac.webkit.org/changeset/238946> All reviewed patches have been landed. Closing bug. |