Bug 79514

Summary: [Chromium] Enable Viewport define by default
Product: WebKit Reporter: Fady Samuel <fsamuel>
Component: New BugsAssignee: Fady Samuel <fsamuel>
Status: RESOLVED FIXED    
Severity: Normal CC: dglazkov, fishd, rjkroege, tdanderson, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
[not for review] alternate implementation?
none
Patch
none
Patch for landing
none
Patch for landing tdanderson: commit-queue-

Description Fady Samuel 2012-02-24 11:20:44 PST
[Chromium] Enable Viewport define by default
Comment 1 Fady Samuel 2012-02-24 11:22:05 PST
Created attachment 128773 [details]
Patch
Comment 2 Fady Samuel 2012-02-24 11:34:21 PST
Decided to only enable for Chromium.
Comment 3 Fady Samuel 2012-02-27 15:06:48 PST
This still needs to be done, but we need to make sure we only enable it on chromium.
Comment 4 Robert Kroeger 2012-02-27 15:17:01 PST
Comment on attachment 128773 [details]
Patch

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

I think you could edit Source/WebKit/chromium/features.gypi to accomplish the same objective? (see the patch)

> Source/JavaScriptCore/wtf/Platform.h:872
> +#define ENABLE_VIEWPORT 1

I think it's preferable to do this a different way.
Comment 5 Robert Kroeger 2012-02-27 15:18:36 PST
Created attachment 129109 [details]
[not for review] alternate implementation?
Comment 6 Fady Samuel 2012-02-28 12:41:27 PST
(In reply to comment #5)
> Created an attachment (id=129109) [details]
> [not for review] alternate implementation?

Looks like this does the trick. Reuploading.
Comment 7 Fady Samuel 2012-02-28 12:46:22 PST
Created attachment 129310 [details]
Patch
Comment 8 WebKit Review Bot 2012-02-28 14:00:57 PST
Comment on attachment 129310 [details]
Patch

Attachment 129310 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/11710074

New failing tests:
platform/chromium/fast/repaint/fixed-layout-360x240.html
inspector/styles/override-screen-size.html
Comment 9 Terry Anderson 2012-03-21 10:44:17 PDT
Created attachment 133073 [details]
Patch for landing
Comment 10 Fady Samuel 2012-03-21 10:45:56 PDT
(In reply to comment #8)
> (From update of attachment 129310 [details])
> Attachment 129310 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/11710074
> 
> New failing tests:
> platform/chromium/fast/repaint/fixed-layout-360x240.html
> inspector/styles/override-screen-size.html

The fixed-layout-360x240 test is obsolete due to a redesign in the way things are done. Deleting it.
Comment 11 WebKit Review Bot 2012-03-21 12:28:09 PDT
Comment on attachment 133073 [details]
Patch for landing

Attachment 133073 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12090536

New failing tests:
inspector/styles/override-screen-size.html
Comment 12 WebKit Review Bot 2012-03-21 13:47:04 PDT
Comment on attachment 133073 [details]
Patch for landing

Attachment 133073 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12071606

New failing tests:
inspector/styles/override-screen-size.html
Comment 13 Fady Samuel 2012-03-21 13:50:03 PDT
(In reply to comment #12)
> (From update of attachment 133073 [details])
> Attachment 133073 [details] did not pass chromium-ews (chromium-xvfb):
> Output: http://queues.webkit.org/results/12071606
> 
> New failing tests:
> inspector/styles/override-screen-size.html

Argh, fixed layout and viewport are tightly coupled on Chromium, which turns out to make things a lot more complicated from a testing standpoint. We're looking into introducing a new flag to make the two operations independently testable.
Comment 14 Terry Anderson 2012-03-28 09:06:09 PDT
Created attachment 134307 [details]
Patch for landing
Comment 15 WebKit Review Bot 2012-03-28 09:09:04 PDT
Attachment 134307 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/plat..." exit_code: 1
ERROR: File does not exist: 'LayoutTests/platform/chromium/fast/repaint/fixed-layout-360x240-expected.png'


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 16 WebKit Review Bot 2012-03-28 10:55:23 PDT
Comment on attachment 129310 [details]
Patch

Clearing flags on attachment: 129310

Committed r112411: <http://trac.webkit.org/changeset/112411>
Comment 17 WebKit Review Bot 2012-03-28 10:55:28 PDT
All reviewed patches have been landed.  Closing bug.