Bug 70370 – Implement CSS Device Adaptation

• Component: CSS
• Status: RESOLVED DUPLICATE

[David Storey]

There is a proposal for doing the viewport meta element via  a CSS @viewport -at-rule instead. It would be nice to have this in WebKit.

Joe Lambert wrote a post which included some rational for wanting something like this in CSS [0]

"Current meta tag support is useful but it does pose some problems, especially if you wish to treat different devices differently. Suppose you’re using media queries to render your site differently for iPhone & iPad, you may wish to prevent scaling on iPhone but not on iPad. This (as far as I have found) isn’t possible without conditionally adding the metatags server side.

"If these viewport controls were controlled via CSS then we could use the same media queries to deliver different settings to different devices!"

[0] http://blog.joelambert.co.uk/2011/10/13/ios5-for-web-developers

[Shane Stephens]

Relevant ED specification: http://dev.w3.org/csswg/css-device-adapt/

[Kenneth Rohde Christiansen]

Assigning to Thiago, as he is working on this.

[Thiago Marcos P. Santos]

Created attachment 123473 [details]
Work in progress of the CSS Device Adaptation

Update on the current status of my implementation: parsing is almost done. I had a problem of properties like zoom, height and width (which is a shorthand in viewport) having a different semantics from other specs. First I though about prefixing like -webkit-viewport-zoom, but that would completely break compatibility with other implementations. I had to create a state whenever the parser is inside a viewport block and go to a different path for property validation.

[WebKit Review Bot]

Attachment 123473 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/css/CSSParser.cpp:683:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/css/CSSParser.cpp:696:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/css/CSSParser.cpp:704:  One space before end of line comments  [whitespace/comments] [5]
Source/WebCore/css/CSSParser.cpp:708:  One space before end of line comments  [whitespace/comments] [5]
Total errors found: 4 in 14 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Gyuyoung Kim]

Comment on attachment 123473 [details]
Work in progress of the CSS Device Adaptation

Attachment 123473 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/11284535

[WebKit Review Bot]

Comment on attachment 123473 [details]
Work in progress of the CSS Device Adaptation

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

[Collabora GTK+ EWS bot]

Comment on attachment 123473 [details]
Work in progress of the CSS Device Adaptation

Attachment 123473 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/11114084

[Kenneth Rohde Christiansen]

Comment on attachment 123473 [details]
Work in progress of the CSS Device Adaptation

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

I am just looking the patch through to get a sense of it, so just some minor comments

> Source/WebCore/ChangeLog:7
> +        Current WIP of CSS Device Adaptation implementation
> +        https://bugs.webkit.org/show_bug.cgi?id=70370
> +
> +        Reviewed by NOBODY (OOPS!).
> +

To ease the pre-review it would be good if you could start by writing a great changelog

> Source/WebCore/css/CSSParser.cpp:662
> +    // Some properties needs remapping because their names clashes with
> +    // other existing properties names but the semantics are different

a nit, in webkit we end our comments with a punctuation mark

> Source/WebCore/css/CSSParser.cpp:678
> +    switch (static_cast<CSSPropertyID>(propId)) {
> +    case CSSPropertyMinWidth:
> +        propId = CSSPropertyWebkitViewportMinWidth;
> +        break;
> +    case CSSPropertyMaxWidth:
> +        propId = CSSPropertyWebkitViewportMaxWidth;
> +        break;
> +    case CSSPropertyMinHeight:
> +        propId = CSSPropertyWebkitViewportMinHeight;
> +        break;
> +    case CSSPropertyMaxHeight:
> +        propId = CSSPropertyWebkitViewportMaxHeight;
> +        break;
> +    case CSSPropertyZoom:
> +        propId = CSSPropertyWebkitViewportZoom;
> +    }

What if it is none of these (new ones could be added in the future)?

> Source/WebCore/css/CSSParser.cpp:703
> +    case CSSPropertyMaxZoom:
> +    case CSSPropertyWebkitViewportZoom:
> +        if (id == CSSValueAuto)
> +            validPrimitive = true;
> +        else
> +            validPrimitive = (!id && validUnit(value, FNumber | FPercent | FNonNeg, m_strict));
> +        break;

Maybe add some comment that MinZoom, MaxZoom and ViewportZoom are handled alike

> Source/WebCore/css/CSSParser.cpp:8114
> +{
> +    ASSERT(!m_inViewportRule);
> +    m_inViewportRule = true;

cant this happen in illegal css code? (just wondering, im no css guru :-)

> Source/WebCore/css/WebKitCSSViewportRule.cpp:51
> +    String result = "@-webkit-viewport { ";
> +    result += m_style->cssText();
> +    result += "}";

There should be a string building in webkit which might be more effective

[Thiago Marcos P. Santos]

Created attachment 149954 [details]
Work in progress of the CSS Device Adaptation v2

The current TODO list for this patch is:

- Add a feature flag, like CSS_DEVICE_ADAPTATION.
- Add js bindings.
- Add layout tests.
- Add the new files to xcode build system.
- Currently viewport tags inside media queries affected by viewport size are ignore (to avoid circular dependencies).
- em units are not supported.
- The resolution property is not implemented is the only property not implemented.

The current status is pretty functional on the Qt port and it passes on the IE10 test suite.
http://samples.msdn.microsoft.com/ietestcenter/default.htm#css3deviceadaptation

The patch is a squash of my personal git branch, which is more readable IMO.
http://www.tmpsantos.com.br/cgi-bin/gitweb.cgi?p=webkit.git;a=shortlog;h=refs/heads/css_device_adaptation

Should I make this bug a [meta] and split this task?

Comments are welcome.

[Build Bot]

Comment on attachment 149954 [details]
Work in progress of the CSS Device Adaptation v2

Attachment 149954 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13111077

[Build Bot]

Comment on attachment 149954 [details]
Work in progress of the CSS Device Adaptation v2

Attachment 149954 [details] did not pass win-ews (win):
Output: http://queues.webkit.org/results/13119030

[WebKit Review Bot]

Comment on attachment 149954 [details]
Work in progress of the CSS Device Adaptation v2

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

[Thiago Marcos P. Santos]

Created attachment 150167 [details]
Work in progress of the CSS Device Adaptation v3

[Thiago Marcos P. Santos]

Created attachment 150182 [details]
Work in progress of the CSS Device Adaptation v4

[WebKit Review Bot]

Comment on attachment 150182 [details]
Work in progress of the CSS Device Adaptation v4

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

[Build Bot]

Comment on attachment 150182 [details]
Work in progress of the CSS Device Adaptation v4

Attachment 150182 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13113409

[Thiago Marcos P. Santos]

Created attachment 150250 [details]
Work in progress of the CSS Device Adaptation v5

[Build Bot]

Comment on attachment 150250 [details]
Work in progress of the CSS Device Adaptation v5

Attachment 150250 [details] did not pass mac-ews (mac):
Output: http://queues.webkit.org/results/13106542

[WebKit Review Bot]

Comment on attachment 150250 [details]
Work in progress of the CSS Device Adaptation v5

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

New failing tests:
dom/xhtml/level3/core/nodeisequalnode22.xhtml
dom/xhtml/level1/core/hc_nodeinsertbeforenewchilddiffdocument.xhtml
dom/xhtml/level3/core/nodeinsertbefore13.xhtml
dom/xhtml/level3/core/nodereplacechild37.xhtml
dom/xhtml/level1/core/hc_attrappendchild5.xhtml
dom/xhtml/level3/core/nodeisequalnode21.xhtml
dom/xhtml/level3/core/nodeinsertbefore08.xhtml
dom/xhtml/level3/core/nodeisequalnode11.xhtml
dom/html/level1/core/hc_elementwrongdocumenterr.html
dom/xhtml/level3/core/nodeisequalnode07.xhtml
dom/html/level1/core/hc_attrinsertbefore6.html
dom/html/level1/core/hc_attrappendchild5.html
dom/html/level1/core/hc_nodeappendchildnewchilddiffdocument.html
dom/html/level1/core/hc_namednodemapwrongdocumenterr.html
dom/xhtml/level3/core/documentadoptnode32.xhtml
dom/xhtml/level1/core/hc_nodeappendchildnewchilddiffdocument.xhtml
dom/xhtml/level3/core/documentadoptnode14.xhtml
dom/xhtml/level1/core/hc_namednodemapwrongdocumenterr.xhtml
dom/html/level1/core/hc_nodeinsertbeforenewchilddiffdocument.html
dom/xhtml/level3/core/nodeissamenode01.xhtml
dom/xhtml/level1/core/hc_attrinsertbefore6.xhtml
dom/xhtml/level1/core/hc_nodereplacechildnewchilddiffdocument.xhtml
dom/xhtml/level3/core/nodeinsertbefore07.xhtml
dom/xhtml/level1/core/hc_elementwrongdocumenterr.xhtml
dom/xhtml/level3/core/nodeinsertbefore23.xhtml
dom/xhtml/level3/core/nodeisequalnode01.xhtml
dom/html/level1/core/hc_nodereplacechildnewchilddiffdocument.html

[WebKit Review Bot]

Created attachment 150275 [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Thiago Marcos P. Santos]

Moving to a [Meta].

*** This bug has been marked as a duplicate of bug 95959 ***