Closed Bug 1349326 Opened 5 years ago Closed 5 years ago

Resolve computed value of "justify-items: auto" more eagerly


(Core :: CSS Parsing and Computation, enhancement)

Not set



Tracking Status
firefox55 --- affected


(Reporter: dholbert, Unassigned)



(PREFACE: this bug is *specifically* about "justify-ITEMS: auto", NOT about "justify-SELF: auto". These properties are easy to confuse, particularly given that they both have magical "auto" values.)

Currently, we resolve "justify-items: auto" to an actual computed value *lazily*, via the "ComputedJustifyItems()" accessor in nsStyleStruct.h.  This is somewhat hacky and problematic, in part because it means this accessor has to walk the nsStyleContext parent chain in some cases, which it seems we can't do in stylo. (See bug 1322570)

I suspect we could just resolve "justify-items:auto" more eagerly in nsRuleNode.cpp, though, and store the actual resolved value in the style struct.  

We initially *had* to do the lazy-resolving thing, because the computed value used to depend on other properties -- notably "display". The spec text used to say:
 # auto
 #  If the inherited value of 'justify-items' includes the
 #  'legacy' keyword, 'auto' computes to the the inherited value.
 #  Otherwise, 'auto' computes to:
 #   * 'stretch' for flex containers and grid containers
 #   * 'start' for everything else

...which we implemented here in the first version of this ComputedJustifyItems function (note the "IsFlexOrGridDisplayType" check):

BUT, the spec has been simplified since then, so that it now only depends on the inherited value of the same property:
 # auto
 #  If the inherited value of 'justify-items' includes the
 #  'legacy' keyword, 'auto' computes to the inherited value.
 #  Otherwise, 'auto' computes to 'normal'.

Note the simplified "Otherwise" clause in the updated spec text.
I'll just take this, since I think the fix is pretty quick, and I think (hope) I've got the relevant confusing spec-text & relationships paged into my head.
Assignee: nobody → dholbert
Just kidding, I think we can't do this after all. :-/  I'd kinda forgotten how style struct sharing works, and now that I've refreshed my memory, I think we're stuck with our current lazy-resolution behavior.  The spec change made the lazy-resolution simpler, but it didn't make it unnecessary.

I was thinking we could eagerly resolve away "justify-items:auto" in nsRuleNode::ComputePositionData, BUT we aren't actually guaranteed that that method will be called.  In particular: if all of the nsStylePosition properties are unspecified, then we'll simply have a *default-constructed* nsStylePosition struct (which may be shared among elements with different style contexts).  And there's no way to know what the correct initial "justify-items" value is for a particular element in the nsStylePosition default constructor, because we don't know what element we're inheriting from there.  So we're stuck with using "auto" (NS_STYLE_JUSTIFY_AUTO) there, and we never get a chance to fix it up because (again) this style struct may be shared among many elements that don't specify any of the nsStylePosition properties.
Assignee: dholbert → nobody
Closed: 5 years ago
Resolution: --- → INVALID
We should figure out how we're going to make this work in Stylo....
I'm pretty much trying to do this in bug 1384542. I think it's doable keeping both the specified-ish value, and the computed value, but let's way for review...
See Also: → 1384542
You need to log in before you can comment on or make changes to this bug.