| Summary: | Remove support for "desktop-width" in the viewport meta tag | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Kenneth Rohde Christiansen <kenneth> | ||||||||||
| Component: | WebCore Misc. | Assignee: | Kenneth Rohde Christiansen <kenneth> | ||||||||||
| Status: | RESOLVED FIXED | ||||||||||||
| Severity: | Normal | CC: | ddkilzer, dglazkov, fsamuel, hausmann, jkjiang, joepeck, johnme, koivisto, kpiascik, mifenton, peter, peter+ews, rwlbuis, simon.fraser, tmpsantos, tonikitoo, webkit.review.bot | ||||||||||
| Priority: | P2 | Keywords: | WebExposed | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| Bug Depends on: | |||||||||||||
| Bug Blocks: | 95959 | ||||||||||||
| Attachments: |
|
||||||||||||
|
Description
Kenneth Rohde Christiansen
2012-11-05 07:03:19 PST
Created attachment 172329 [details]
Patch
Comment on attachment 172329 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=172329&action=review Removing this feature seems fine to me. Thanks Kenneth! > Source/WebCore/dom/ViewportArguments.cpp:-219 > - if (equalIgnoringCase(valueString, "desktop-width")) We'll probably want to remove parsing in findScaleValue() and findUserScalableValue() as well. Comment on attachment 172329 [details] Patch Attachment 172329 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14730071 Created attachment 172337 [details]
Patch
The Chromium port is using this in Source/WebKit/chromium/src/ChromeClientImpl.cpp, in ChromeClientImpl::dispatchViewportPropertiesDidChange(). Passing in a default viewport apparently is a signal to reset it, which sets the args.width value to DesktopWidth. Since arguments.initialScale defaults to Auto and this *is* a default viewport, we should be ending up setting width=desktopWidth on line 136 of ViewportArguments.cpp anyway. It looks like it should be fine to remove lines 618-623 from ChromeClientImpl.cpp Fady, could you confirm this? Comment on attachment 172337 [details] Patch Attachment 172337 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14728622 Comment on attachment 172337 [details] Patch Attachment 172337 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14747075 Created attachment 172567 [details]
Patch
Comment on attachment 172567 [details]
Patch
You're missing a change to Source/WebKit/blackberry/Api/WebViewportArguments.h which also makes use of ValueDesktopWidth. It's defined it its own enum, so this patch won't break anything for our port. I can easily follow up with a patch if you want to go ahead and land this though.
Comment on attachment 172567 [details] Patch Attachment 172567 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14728837 Comment on attachment 172567 [details] Patch Attachment 172567 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14747472 Created attachment 172611 [details]
Patch
Comment on attachment 172611 [details] Patch Clearing flags on attachment: 172611 Committed r133729: <http://trac.webkit.org/changeset/133729> All reviewed patches have been landed. Closing bug. |