RESOLVED FIXED 101217
Remove support for "desktop-width" in the viewport meta tag 
https://bugs.webkit.org/show_bug.cgi?id=101217
Summary Remove support for "desktop-width" in the viewport meta tag
Kenneth Rohde Christiansen
Reported 2012-11-05 07:03:19 PST
This has been removed from the CSS Device Adaptation spec, and should be safe to remove as it is not documented anywhere (neither in blog posts).
Attachments
Patch (3.01 KB, patch)
2012-11-05 07:04 PST, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
Patch (3.89 KB, patch)
2012-11-05 08:07 PST, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
Patch (5.83 KB, patch)
2012-11-06 06:17 PST, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
Patch (7.83 KB, patch)
2012-11-06 10:28 PST, Kenneth Rohde Christiansen 		no flags
DetailsFormatted DiffDiff
Show Obsolete (3) View All Add attachment proposed patch, testcase, etc.
Kenneth Rohde Christiansen
Comment 1 2012-11-05 07:04:56 PST
Created attachment 172329 [details] Patch
Peter Beverloo
Comment 2 2012-11-05 07:23:00 PST
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.
WebKit Review Bot
Comment 3 2012-11-05 07:57:29 PST
Comment on attachment 172329 [details] Patch Attachment 172329 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14730071
Kenneth Rohde Christiansen
Comment 4 2012-11-05 08:07:58 PST
Created attachment 172337 [details] Patch
Peter Beverloo
Comment 5 2012-11-05 08:16:39 PST
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?
WebKit Review Bot
Comment 6 2012-11-05 08:53:00 PST
Comment on attachment 172337 [details] Patch Attachment 172337 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14728622
Peter Beverloo (cr-android ews)
Comment 7 2012-11-05 09:28:37 PST
Comment on attachment 172337 [details] Patch Attachment 172337 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14747075
Kenneth Rohde Christiansen
Comment 8 2012-11-06 06:17:58 PST
Created attachment 172567 [details] Patch
Konrad Piascik
Comment 9 2012-11-06 06:35:51 PST
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.
Peter Beverloo (cr-android ews)
Comment 10 2012-11-06 07:48:41 PST
Comment on attachment 172567 [details] Patch Attachment 172567 [details] did not pass cr-android-ews (chromium-android): Output: http://queues.webkit.org/results/14728837
WebKit Review Bot
Comment 11 2012-11-06 10:09:05 PST
Comment on attachment 172567 [details] Patch Attachment 172567 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/14747472
Kenneth Rohde Christiansen
Comment 12 2012-11-06 10:28:16 PST
Created attachment 172611 [details] Patch
WebKit Review Bot
Comment 13 2012-11-07 00:55:13 PST
Comment on attachment 172611 [details] Patch Clearing flags on attachment: 172611 Committed r133729: <http://trac.webkit.org/changeset/133729>
WebKit Review Bot
Comment 14 2012-11-07 00:55:19 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.