stylo: implement grid property glue

REOPENED
Assigned to

Status

()

Core
CSS Parsing and Computation
P1
normal
REOPENED
3 months ago
4 days ago

People

(Reporter: bz, Assigned: waffles)

Tracking

(Blocks: 4 bugs)

Trunk
mozilla55
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox55 fixed)

Details

MozReview Requests

()

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(1 attachment)

Pretty much all the reftests in layout/reftests/css-grid are marked failing.
Priority: -- → P2
This is because most of the grid properties aren't yet implemented, but this is underway, see:

https://github.com/servo/servo/pull/15628
https://github.com/servo/servo/pull/16081
Summary: stylo: need to make grid support actually work → stylo: implement grid property glue
https://github.com/servo/servo/issues/15307 is the metabug, though that may only be for parsing / serialization.
Blocks: 1243581
Blocks: 1356087
Waffles, you're working on this right? How much left is there to do here?
Assignee: nobody → wafflespeanut
Flags: needinfo?(wafflespeanut)
Priority: P2 → P1
(Assignee)

Comment 4

2 months ago
All the longhands other than `grid-template-areas` have been implemented ( grid-template-{rows,columns} will land soon - https://github.com/servo/servo/pull/16067 ). Though the metabug lists only parsing/serialization, I made sure that the gecko glue was also implemented along the way :)

None of the shorthands have been implemented yet.
Flags: needinfo?(wafflespeanut)
Great - thanks for the update! Let me know if you get blocked on anything, the grid stuff is pretty high priority.
Comment hidden (mozreview-request)
We needed to update the test expectations after https://github.com/servo/servo/pull/16443. But I need to wait for someone to push it since I can't do it myself :)

Comment 8

a month ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/337f30ecca23
stylo: Update reftest expectations for grid-{row,column} and grid-area. r=test-annotations-update

Comment 9

a month ago
Pushed by archaeopteryx@coole-files.de:
https://hg.mozilla.org/integration/autoland/rev/5f4fc7d22293
stylo: Update reftest expectations for grid-{row,column} and grid-area. r=test-annotations-fix
https://hg.mozilla.org/mozilla-central/rev/337f30ecca23
https://hg.mozilla.org/mozilla-central/rev/5f4fc7d22293
Status: NEW → RESOLVED
Last Resolved: a month ago
status-firefox55: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
(Assignee)

Updated

21 days ago
Depends on: 1362843
It seems to me grid support for stylo is still far from complete. Reopening.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
:waffles has a PR open that I need to review and carry, then there will be very few things remaining.
(Assignee)

Comment 13

9 days ago
Only grid and grid-template shorthands remain now.
Blocks: 1339656
status-firefox54: affected → ---
(Assignee)

Comment 14

4 days ago
Once https://github.com/servo/servo/pull/17021/ lands, we'll have all grid properties (only a few bugs will remain).
(Assignee)

Comment 15

4 days ago
We still need to decide whether we should support subgrid grammar (which is based on old spec and it's prefs-gated in gecko), because many failures are on subgrid stuff.
(In reply to Ravi Shankar [:waffles] from comment #15)
> We still need to decide whether we should support subgrid grammar (which is
> based on old spec and it's prefs-gated in gecko), because many failures are
> on subgrid stuff.

How much work is it to mimic Gecko here (including the pref-gating)? In general that should be the default, because it's the lowest risk.
(Assignee)

Comment 17

4 days ago
Not much, really. I could add it straightaway. Just wondering whether we should implement something that doesn't even exist in the spec anymore :)
(In reply to Ravi Shankar [:waffles] from comment #17)
> Not much, really. I could add it straightaway. Just wondering whether we
> should implement something that doesn't even exist in the spec anymore :)

We should, so please do!
This preference is off by default in current Firefox. Do we really need to add that cruft to Servo?
(In reply to Ravi Shankar [:waffles] from comment #17)
> Just wondering whether we should implement something that doesn't even exist in the spec anymore :)

It's still specced, but it's just in a different level of the spec:
  https://drafts.csswg.org/css-grid-2/#subgrids
(It was moved per CSSWG resolution noted here: https://github.com/w3c/csswg-drafts/issues/958#issuecomment-286813071 )

Authors & spec editors are very adamant about "subgrid" as a feature, but the specifics may change as CSS Grid Level 2 is fleshed out.

Mats can comment more about timeline for implementing it in layout (my impression is that it's not on his front-burner yet & may not be for a little while).  So IMO it'd be OK to deprioritize "subgrid" parsing support in Stylo for the moment, and behave as if this pref simply doesn't exist (and tag the corresponding tests as "fails-if(stylo)" or whatever).
You need to log in before you can comment on or make changes to this bug.