Summary: | Implement CSS Device Adaptation | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | David Storey <dstorey> | ||||||||||||||
Component: | CSS | Assignee: | Thiago Marcos P. Santos <tmpsantos> | ||||||||||||||
Status: | RESOLVED DUPLICATE | ||||||||||||||||
Severity: | Enhancement | CC: | abarth, ap, cmarcelo, ddkilzer, dglazkov, dino, donggwan.kim, fsamuel, gustavo.noronha, gustavo, haraken, hausmann, japhet, jochen, joepeck, johnme, kenneth, kling, koivisto, macpherson, menard, mike, mjs, peter, phiw2, rakuco, rbyers, rob, sam, shanestephens, simon.fraser, siraj, tonikitoo, webkit.review.bot, xan.lopez, yael, zalan | ||||||||||||||
Priority: | P2 | ||||||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||
OS: | Unspecified | ||||||||||||||||
URL: | http://dev.w3.org/csswg/css-device-adapt/ | ||||||||||||||||
Attachments: |
|
Description
David Storey
2011-10-18 14:58:23 PDT
Relevant ED specification: http://dev.w3.org/csswg/css-device-adapt/ Assigning to Thiago, as he is working on this. 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.
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.
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 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 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 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 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. 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 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 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 Created attachment 150167 [details]
Work in progress of the CSS Device Adaptation v3
Created attachment 150182 [details]
Work in progress of the CSS Device Adaptation v4
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 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 Created attachment 150250 [details]
Work in progress of the CSS Device Adaptation v5
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 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 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
Moving to a [Meta]. *** This bug has been marked as a duplicate of bug 95959 *** |