Bug 79514 – [Chromium] Enable Viewport define by default

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Fady Samuel]

[Chromium] Enable Viewport define by default

[Fady Samuel]

Created attachment 128773 [details]
Patch

[Fady Samuel]

Decided to only enable for Chromium.

[Fady Samuel]

This still needs to be done, but we need to make sure we only enable it on chromium.

[Robert Kroeger]

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.

[Robert Kroeger]

Created attachment 129109 [details]
[not for review] alternate implementation?

[Fady Samuel]

(In reply to comment #5)
> Created an attachment (id=129109) [details]
> [not for review] alternate implementation?

Looks like this does the trick. Reuploading.

[Fady Samuel]

Created attachment 129310 [details]
Patch

[WebKit Review Bot]

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

[Terry Anderson]

Created attachment 133073 [details]
Patch for landing

[Fady Samuel]

(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.

[WebKit Review Bot]

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

[WebKit Review Bot]

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

[Fady Samuel]

(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.

[Terry Anderson]

Created attachment 134307 [details]
Patch for landing

[WebKit Review Bot]

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.

[WebKit Review Bot]

Comment on attachment 129310 [details]
Patch

Clearing flags on attachment: 129310

Committed r112411: <http://trac.webkit.org/changeset/112411>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.