1520684 - [css-logical] Implement the flow-relative border-radius properties

Status: RESOLVED FIXED

(Core :: CSS Parsing and Computation, enhancement, P3)

Description

[Mats Palmgren (inactive)]

https://drafts.csswg.org/css-logical-1/#border-radius-properties

Comment 1

[Mats Palmgren (inactive)]

Attachment: part 1 - Add plumbing for handling physical/logical corner values

Comment 2

[Mats Palmgren (inactive)]

Attachment: part 2 - [css-logical] Implement the flow-relative border-radius properties

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 9037346 [details] [diff] [review]
part 1 - Add plumbing for handling physical/logical corner values

Review of attachment 9037346 [details] [diff] [review]:
-----------------------------------------------------------------

::: servo/components/style/logical_geometry.rs
@@ +171,5 @@
>          }
>      }
>  
>      #[inline]
> +    pub fn physical_sides_to_corner(block_side: PhysicalSide, inline_side: PhysicalSide) -> PhysicalCorner {

nit: Let's not make this `pub`.

@@ +187,5 @@
> +    }
> +
> +    #[inline]
> +    pub fn start_start_physical_corner(&self) -> PhysicalCorner {
> +        WritingMode::physical_sides_to_corner(self.block_start_physical_side(),

nit: Let's use block indentation like the rest of the file, so:

WritingMode::physical_sides_to_corner(
    self.block_start_physical_side(),
    self.inline_start_physical_side(),
)

here and elsewhere.

::: servo/components/style/properties/helpers.mako.rs
@@ +841,5 @@
>  <%def name="logical_setter_helper(name)">
>      <%
>          side = None
>          size = None
> +        corner = None

Ugh, I did not remember how ugly this code was, damn :(

Comment 4

[Mats Palmgren (inactive)]

Intent-to-ship: https://groups.google.com/forum/#!topic/mozilla.dev.platform/Pjf2a6t-sFA

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/c39aa6dfd458
part 1 - Add plumbing for handling physical/logical corner values.  r=emilio
https://hg.mozilla.org/integration/mozilla-inbound/rev/2406b20fb105
part 2 - [css-logical] Implement the flow-relative border-radius properties.  r=emilio

Comment 6

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/c39aa6dfd458
https://hg.mozilla.org/mozilla-central/rev/2406b20fb105

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/web-platform-tests/wpt/pull/14955 for changes under testing/web-platform/tests

Comment 8

[Chris Mills [:cmills]]

Docs completed; Rachel Andrew created the docs a while ago, and recently updated the compat data too:

https://developer.mozilla.org/en-US/docs/Web/CSS/border-end-end-radius#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/border-end-start-radius#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/border-start-end-radius#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/CSS/border-start-start-radius#Browser_compatibility

I also added a note to the Fx66 rel notes:

https://developer.mozilla.org/en-US/docs/Mozilla/Firefox/Releases/66#CSS

Comment 9

[Oriol Brufau [:Oriol]]

https://developer.mozilla.org/en-US/docs/Web/CSS/border-start-end-radius says

For instance, in a horizontal-tb writing mode this property corresponds to the border-bottom-left-radius property.

Actually, in horizontal-tb ltr, it corresponds to border-top-right-radius.

https://developer.mozilla.org/en-US/docs/Web/CSS/border-end-start-radius is also wrong.

Not surprising, since the spec doesn't define the mapping: https://github.com/w3c/csswg-drafts/issues/3034