Summary: | [css-logical] Flow relative margin, padding, border and sizing properties | ||
---|---|---|---|
Product: | WebKit | Reporter: | Oriol Brufau <obrufau> |
Component: | CSS | Assignee: | Oriol Brufau <obrufau> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | commit-queue, dbates, ews-watchlist, jfernandez, rego, rniwa, rwlbuis, webkit-bug-importer |
Priority: | P2 | Keywords: | InRadar |
Version: | WebKit Nightly Build | ||
Hardware: | Unspecified | ||
OS: | Unspecified | ||
See Also: | https://bugs.chromium.org/p/chromium/issues/detail?id=850000 | ||
Bug Depends on: | |||
Bug Blocks: | 185977 | ||
Attachments: |
Description
Oriol Brufau
2018-08-07 12:02:03 PDT
Created attachment 346764 [details]
Patch
Comment on attachment 346764 [details] Patch Attachment 346764 [details] did not pass mac-ews (mac): Output: https://webkit-queues.webkit.org/results/8797759 New failing tests: fast/css/style-enumerate-properties.html Created attachment 346765 [details]
Archive of layout-test-results from ews103 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-ews.
Bot: ews103 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 346764 [details] Patch Attachment 346764 [details] did not pass mac-wk2-ews (mac-wk2): Output: https://webkit-queues.webkit.org/results/8797779 New failing tests: fast/css/style-enumerate-properties.html Created attachment 346766 [details]
Archive of layout-test-results from ews106 for mac-sierra-wk2
The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106 Port: mac-sierra-wk2 Platform: Mac OS X 10.12.6
Comment on attachment 346764 [details] Patch Attachment 346764 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/8797822 New failing tests: fast/css/style-enumerate-properties.html Created attachment 346767 [details]
Archive of layout-test-results from ews113 for mac-sierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews113 Port: mac-sierra Platform: Mac OS X 10.12.6
Comment on attachment 346764 [details] Patch Attachment 346764 [details] did not pass ios-sim-ews (ios-simulator-wk2): Output: https://webkit-queues.webkit.org/results/8797841 New failing tests: fast/css/style-enumerate-properties.html Created attachment 346768 [details]
Archive of layout-test-results from ews125 for ios-simulator-wk2
The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews125 Port: ios-simulator-wk2 Platform: Mac OS X 10.13.4
Comment on attachment 346764 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=346764&action=review It looks good but I think it still needs some minor changes. Apart from that you should fix the test afilures reported by the EWSs. > Source/WebCore/ChangeLog:35 > + (WebCore::isLayoutDependent): Please document this change here, as this is something special. > Source/WebCore/css/CSSProperties.json:3069 > - "category": "css-22", > - "url": "https://www.w3.org/TR/CSS22/box.html#propdef-padding" > + "category": "css-22", > + "url": "https://www.w3.org/TR/CSS22/box.html#propdef-padding" What's the change in these lines? > LayoutTests/imported/w3c/ChangeLog:14 > + These properties provide the author with the ability to control margins > + through logical, rather than physical, direction and dimension mappings. > + > + Only longhand properties and border shorthands for specific sides are > + implemented as part of this patch. > + > + The existing prefixed properties become aliases of the new ones. Probably this specific ChangeLog (the one in "LayoutTests/imported/w3c/") should mention that you're importing the WPT test suite for CSS Logical Properties and Values spec. > LayoutTests/imported/w3c/web-platform-tests/css/css-logical/animation-001-expected.txt:2 > +FAIL Logical properties can be animated using object notation assert_equals: expected "50px" but got "0px" It'd be nice to report a bug about these failures. > LayoutTests/imported/w3c/web-platform-tests/css/css-logical/logical-box-border-color-expected.txt:15 > +FAIL Test that logical border-*-color properties share computed values with their physical associates, with 'writing-mode: sideways-rl; direction: rtl; '. assert_equals: logical properties on one declaration, writing mode properties on another, 'writing-mode: sideways-rl; direction: rtl; ', border-bottom-color expected "rgb(1, 1, 1)" but got "rgb(4, 4, 4)" Add a comment on ChangeLog about all sideways tests failing as they're not supported. > LayoutTests/imported/w3c/web-platform-tests/css/css-logical/logical-box-margin-expected.txt:4 > +FAIL Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: horizontal-tb; direction: ltr; '. assert_equals: shorthand properties on one declaration, writing mode properties on another, 'writing-mode: horizontal-tb; direction: ltr; ', margin-inline-start expected "1px" but got "0px" Why this stuff is failing? (In reply to Manuel Rego Casasnovas from comment #10) > > Source/WebCore/css/CSSProperties.json:3069 > > - "category": "css-22", > > - "url": "https://www.w3.org/TR/CSS22/box.html#propdef-padding" > > + "category": "css-22", > > + "url": "https://www.w3.org/TR/CSS22/box.html#propdef-padding" > > What's the change in these lines? No change. I don't understand why it's marked as such. > > LayoutTests/imported/w3c/web-platform-tests/css/css-logical/logical-box-margin-expected.txt:4 > > +FAIL Test that margin-* shorthands set the computed value of both logical and physical longhands, with 'writing-mode: horizontal-tb; direction: ltr; '. assert_equals: shorthand properties on one declaration, writing mode properties on another, 'writing-mode: horizontal-tb; direction: ltr; ', margin-inline-start expected "1px" but got "0px" > > Why this stuff is failing? Because margin-block and margin-inline have not been implemented yet. I checked that the test results are the same as in Chromium without experimental web features. Created attachment 346773 [details]
Patch
Comment on attachment 346773 [details]
Patch
Thanks, this LGTM but please wait until Monday for landing (as you said on the mail to webkit-dev).
Comment on attachment 346773 [details] Patch Clearing flags on attachment: 346773 Committed r234798: <https://trac.webkit.org/changeset/234798> All reviewed patches have been landed. Closing bug. |