[Webkit-unassigned] [Bug 188386] [css-logical] Flow relative margin, padding, border and sizing properties

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=188386

Manuel Rego Casasnovas <rego at igalia.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #346764|review?                     |review-
              Flags|                            |

--- Comment #10 from Manuel Rego Casasnovas <rego at igalia.com> ---
Comment on attachment 346764
  --> https://bugs.webkit.org/attachment.cgi?id=346764
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?

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.webkit.org/pipermail/webkit-unassigned/attachments/20180808/b131f275/attachment.html>