2005007 - Add parsing for CSS corner-shape support

Status: ASSIGNED

(Core :: CSS Parsing and Computation, enhancement)

Description

[Daniel Holbert [:dholbert]]

Filing a bug for corner-shape CSS parsing, as a building-block towards full support for the feature (tracked in bug 1726232).

https://drafts.csswg.org/css-borders-4/#corner-shaping-shorthand

Comment 1

[Mauro V [:cheff]]

Attachment: Bug 2005007 - Initial implementation for corner-shape r=#firefox-style-system-reviewers

Comment 2

[Daniel Holbert [:dholbert]]

Sorry, assignee got automatically set when I transferred the patch over. Unassigning to restore the prior state of things (cheff unassigned on the main bug in https://bugzilla.mozilla.org/show_bug.cgi?id=1726232#a133540423_766482 ); but cheff, if you'd like to push this forward more, please feel free. :) thanks!

Comment 3

[Mauro V [:cheff]]

I tried to work on this but gave up in hopes that someone will take over (I still have so much to learn lol). Sorry

Comment 4

[Daniel Holbert [:dholbert]]

no worries. Thanks!

Comment 5

[Mauro V [:cheff]]

Is there an estimate on when firefox is planning on adding this feature by the way?

Comment 6

[Daniel Holbert [:dholbert]]

No estimate at the moment. (It's nominated for interop-2026, and if it ends up being part of that, then it'd arrive at some point in 2026. Or it might arrive during 2026 regardless; we'll see.)

Comment 7

[Mauro V [:cheff]]

I managed to get it parsing and rendering! I wonder if I could submit it all in a single bug (https://bugzilla.mozilla.org/show_bug.cgi?id=1726232) so I dont have to deal with splitting it up into patches?

Comment 8

[Daniel Holbert [:dholbert]]

Let's not do that, for several reasons:

1. Bug 1726232 is being used as a tracker-bug, and we'll use its dependency-list as a way to track followup items and newly-discovered issues until (and perhaps after) the implementation is high-quality enough to be shippable. So it's best not to attach any patches there -- even a potentially-complete patch or patch-series -- since we automatically close bugs once their attached patches land, and don't want to close bug 1726232 until we've gotten enough Nightly testing & fixed any identified issues & decided the feature is shippable.

2. Breaking up the patch helps ease cognitive load for reviewers (think about one set of things at a time and sign off on that set of things without having to get through everything)

3. Breaking up the patch helps helps direct the different pieces to different experts (folks reviewing the CSS pieces may not be experts in the webrender clipping/painting pieces)

4. Breaking up the patch lets "finished" parts get checked in, even as issues or open-questions are worked out in later parts. (So hypothetically we could check in your CSS parsing pieces -- behind an off-by-default about:config preference -- without needing to block that on having the rendering bits all done.)

5. It's a useful exercise to split up your patch (for a feature like this) into two or more pieces, as a way of making sure your patch's approach is right; that helps to identify unused parts or parts that don't fit together, and it's a useful bit of "self-review".

I'd suggest splitting this into at least two pieces:
(A) the css parts where we parse the property but it doesn't impact rendering
(B) the rendering parts

Those parts should be touching entirely different sets of files, probably -- and if you've got a working patch, I'd expect you could just look at the files-touched and split them into part (A) or part (B), and generate two patches, without too much trouble.

Comment 9

[Mauro V [:cheff]]

Should I split these 2 patches into this bug or should I make a new bug to track the rendering part? I doubt one will build with the other one... But I could try

Comment 10

[Daniel Holbert [:dholbert]]

Typically we do the CSS parts in one bug, and the rendering parts in another.

I doubt one will build with the other one... But I could try

The first should build on its own, and the second should layer on top of it. (CSS parsing support should work independently of whether you actually use the parsed value for rendering.)

See one example in bug 1901374 where we added support for a new property in CSS, which (at that point) has no code that actually uses it for rendering.

Comment 11

[Mauro V [:cheff]]

Ok thanks, is there a "rendering" bug report im missing or should I create one?

Comment 12

[Daniel Holbert [:dholbert]]

I've just filed bug 2035317 to serve that purpose.

Comment 13

[Mauro V [:cheff]]

Thanks!

Comment 14

[Mauro V [:cheff]]

Attachment: Bug 2005007 - Add parsing for CSS corner-shape support

Comment 15

[Mauro V [:cheff]]

Attachment: Bug 2005007 - Add parsing for CSS corner-shape support

Comment 16

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/5f837dd775a3
https://hg.mozilla.org/integration/autoland/rev/567230e61280
Add parsing for CSS corner-shape support r=firefox-style-system-reviewers,emilio,dshin

Comment 17

[Rémi Papillié [:wsmind]]

Attachment: Bug 2005007 - Remove extra clone(). r=emilio

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/2170282c866a
https://hg.mozilla.org/integration/autoland/rev/ee0edadda433
Remove extra clone(). r=emilio,firefox-style-system-reviewers

Comment 19

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

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