Serialization of border-radius and -moz-outline-radius shorthand is wrong

[upsuper]

For example, border-radius: 1px 2px 3px 4px / 5px 6px 7px 8px is incorrectly serialized as border-radius: 1px 5px 2px 6px 3px 7px 4px 8px.

[highfive]

Please make a comment here if you intend to work on this issue. Thank you!

[upsuper]

Note that, this also affects Gecko-only -moz-outline-radius, which uses basically the same syntax.

[jdm]

<div style="border-radius: 1px 2px 3px 4px / 5px 6px 7px 8px">
<script>console.log(document.querySelector('div').style.borderRadius)</script>

[Aaron-Tang]

Hi, I intend to work on this issue

[upsuper]

Great! Feel free to ask any questions.

[upsuper]

In addition to the / separator, the serialization should also be condensed to the shortest form when possible, which means border-radius: 1px 1px 1px 1px / 2px 2px 2px 2px should be serialized to border-radius: 1px / 2px, and border-radius: 1px 1px 1px 1px / 1px 1px 1px 1px should be border-radius: 1px.

[jdm]

@Aaron-Tang have you made progress? Any questions?

[Aaron-Tang]

Sorry I haven't been able to sit down and work on it that much, I'll probably get to it this weekend. Quick question however, which part of the code should I be working on? There doesn't seem to be anything linked in the issue.

[upsuper]

Relevant code should be https://github.com/servo/servo/blob/master/components/style/properties/shorthand/border.mako.rs#L165-L181

[Aaron-Tang]

Um I have a few questions about the linked code; I'm new to rust and I'm trying to understand the code first:

• What does "fn to_css_declared(&self, dest: &mut W) -> fmt::Result where W: fmt::Write" do?
• Do border_top_left_radius and the other ones have two fields (as in the example you gave above it seems that border_top_left_radius is equal to both 1px and 5px).
• If yes how would i access the fields? Is it like accessing an array value (somelist[2] = some_value) or is there a way unique to rust to do this?

[upsuper]

• to_css_declared serializes the longhand property and writes the result into dest
• border_top_left_radius and others are defined in longhand/border.mako.rs, and their type is BorderRadiusSize.

[wafflespeanut]

@Aaron-Tang ping?

[Aaron-Tang]

Sorry I'm still very confused on how to fix this bug. I understand that the issue now is that the to_css function gets both the height and width of the borderradius into the dest object, which is then written using write. I'm trying to figure out how to access the height and width attributes of BorderRadiusSize before this occurs so I can run checks to see if it can be shortened. I can't seem to find a way to do so. I don't think I mentioned before that I'm totally new to rust which is why a lot of this is confusing to me. Any help would be greatly appreciated.

[wafflespeanut]

You don't really need to worry about everything. Let's go through some stuff. We know that the parsing code which uses the existing BorderRadius implementation works just fine, but the serializing code directly writes these values (separated by spaces, just like the issue says), which is not what we want. The clue lies in the existing ToCss implementation of BorderRadius.

[austinprete]

@wafflespeanut is this issue now open for someone else to pick up? (I saw that you removed the assigned label, and the discussion has been stale for a while) If so, I'd like to work on it, though it'd probably be until next weekend before I'd have time to make progress. (Busy school week for me)

[KiChjang]

@austinprete Let us know when you start working on it then. In the meantime, this issue is still open!

[highfive]

cc @emilio

[fnune]

Hello! I would like to work on this issue. I am new to the language: my only experience is a couple small applications that use APIs from services like Slack, Asana and my company's own backends.

I think with the comments on this wall and some effort I'll be able to tackle it, though!

[jdm]

Great! Please ask questions if anything is unclear!

[wafflespeanut]

Wonderful. I never noticed that this addresses #12655.