First step for this is probably keeping these values around for layout. Bug 1519958 is about that.

The fix here would probably be quite similar the one in bug 1339672, if that helps.

(In reply to Phil McCloghry-Laing from comment #4)

I'm interested in working on a fix for this.

The approach I'm looking at is replacing the RepeatAutoIndex with
RepeatAutoStartIndex and RepeatAutoEndIndex. It means that some of items in
the LineNameLists can be for the repeat track list (where
RepeatAutoStartIndex < index < RepeatAutoEndIndex), similar to the
Min/MaxTrackSizingFunctions.

Hi Phil,
Do you still have interest in this? We also have to update the parser of TrackRepeat (which was wroten in Rust), and update something like ExpandNonRepeatAutoTracks() and others if needed. Thanks.

Hi (In reply to Boris Chiou [:boris] from comment #7)

(In reply to Phil McCloghry-Laing from comment #4)

I'm interested in working on a fix for this.

The approach I'm looking at is replacing the RepeatAutoIndex with
RepeatAutoStartIndex and RepeatAutoEndIndex. It means that some of items in
the LineNameLists can be for the repeat track list (where
RepeatAutoStartIndex < index < RepeatAutoEndIndex), similar to the
Min/MaxTrackSizingFunctions.

Hi Phil,
Do you still have interest in this? We also have to update the parser of TrackRepeat (which was wroten in Rust), and update something like ExpandNonRepeatAutoTracks() and others if needed. Thanks.

I'm still interested in doing this. I did start working on it, including the parsing in rust. I haven't worked on any Mozilla code previously - is someone available to review changes and what's the best way to submit them?

(In reply to Phil McCloghry-Laing from comment #8)

I'm still interested in doing this. I did start working on it, including the parsing in rust. I haven't worked on any Mozilla code previously - is someone available to review changes and what's the best way to submit them?

Great. You will update grid code and part of Rust parser, so you could send the review request to Mats. I think the best way is to use Phabricator for sending patches and reviewing them. There is a document about how to set up: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html.

Once you set it up, you can use git or hg to push your patches into Phabricator, and Bugzilla will display the link automatically. Send the review request in Phabricator, and then I believe the reviewer will handle the rest part.

FTR, we should also fix repeat(auto-fill, <line-names>+) in subgridvalues:
https://searchfox.org/mozilla-central/rev/153feabebc2d13bb4c29ef8adf104ec1ebd246ae/servo/components/style/values/generics/grid.rs#l662
which can have names for multiple lines per:
https://drafts.csswg.org/css-grid-2/#subgrid-per-axis
https://drafts.csswg.org/css-grid-2/#typedef-name-repeat

Hi Phil — Just wanted to check in to see if you are still working on this?

Enable several reftests which expected failure because of this
Update a mochitest which seems to be expecting earlier CSS draft behavior.

The information in it is always derivable from the values of mRepeatAutoStart
and mRepeatAutoEnd. Additionally, its value is used in some cases where it has
not yet been set properly (such as CalculateRepeatFillCount).

This works out currently because the default value is zero we only accept
repeat(auto-fill, ...) and repeat(auto-fit, ...) CSS values that have a single
element in the repeat. In that case, zero is the correct value for
RepeatEndDelta.

https://hg.mozilla.org/mozilla-central/rev/e5245d585b9a

https://hg.mozilla.org/mozilla-central/rev/4a3321b64a58

I'm a bit confused about this patch set. D55725 makes a bunch of changes to nsComputedDOMStyle.cpp which appears to be very similar to the changes in D59547. Could you rebase the patch set on current m-c and tell me which order I should apply these patches please?

(FYI, it's common practice to add "part 1" "part 2" etc to the patches to indicate the order and also to highlight in the commit log that a patch is part of a series of related changes.)

(Also, a minor typo in the last patch summary: s/more than value/more than one value/)

Sorry about that, I should have made what I was trying to do more clear.

I'm trying to break some of D55725 into smaller parts, and also make it so that it's easier to just "flip the switch" in servo when everything else is ready.

That's the idea behind D59070 and D59547. Those changes are supposed to be safe to make even if we aren't getting multiple repeat values from CSS yet.

I will also mark those as "part 1"/"part 2" etc.

This also means that the result of CalculateRepeatFillCount is specified to be
a number of repetitions of all repeat tracks, not the total number of tracks.

The test to ensure only a single repeat value is allowed will be removed by a later commit.

This includes when using subgrid layout.

To complete support, also finish the TODO about this in nsGridContainerFrame
when calculating track sizes and properly enumerate repeat values in the
computed dom.

After applying all the patches, I see the following results in the devtools Console:

[x a b] 50px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 50px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 50px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 50px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 50px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 50px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 50px [b a b] 0px [b a b] 0px [b a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 50px [b a b] 0px [b a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 50px [b a b] 0px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [b a b] 50px [b a b] 0px [b a b] 0px [d e y]
[x a b] 0px [b a b] 0px [b a b] 0px [b a b] 0px [b a b] 50px [b a b] 0px [d e y]

whereas Chrome Canary says:

[x a b] 50px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 100px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 150px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 50px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 100px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 150px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 50px [b] 0px [c] 0px [d e a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 100px [c] 0px [d e a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 150px [d e a b] 0px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e a b] 50px [b] 0px [c] 0px [d e y]
[x a b] 0px [b] 0px [c] 0px [d e a b] 0px [b] 100px [c] 0px [d e y]

Chrome's results looks correct to me.

So I added a few tests to testing/web-platform/tests/css/css-grid/subgrid/repeat-auto-fill-008.html and I'm not sure I agree with the current results. A repeat(auto-fill) with multiple values in the non-subgrid case only repeats if all the values fit - shouldn't subgrid do the same?
That's my understanding of the spec text here, fwiw:
https://drafts.csswg.org/css-grid-2/#typedef-line-name-list

For example, given a subgrid that spans four tracks and has subgrid repeat(auto-fill, [x] [y]) [z] [z], shouldn't the used value be subgrid [x] [y] [z] [z] since we can only repeat all the auto-fill names once? (repeating it once more: subgrid [x] [y] [x] [y] [z] [z] is clearly invalid since that implies we have five tracks, and the value I get with these patches subgrid [x] [y] [x] [z] [z] also seems invalid since the 2nd repetition isn't complete)

You are right, I am looking at fixing that now. I think the problem is because the constructor for LineNameMap when using subgrid doesn't ensure the difference between mRepeatStart and mRepeatEnd is a multiple of the repeat's length, although I'm not entirely certain that will be sufficient. If you have a better solution (or don't think that's the right place to handle that) let me know.

I will update the tests for bug 1613725 to include those cases.

Actually, for some of those cases I see you have fewer than the span of tracks in the expectations. Shouldn't the remaining tracks be filled with empty name lists?

Oh right, I forgot that we fill those out for subgrid.

Backed out 7 changesets (Bug 1341507) for mochitest failures in dom/grid/test/chrome/test_grid_repeat_auto_fill.html CLOSED TREE

https://treeherder.mozilla.org/#/jobs?repo=autoland&resultStatus=testfailed%2Cbusted%2Cexception&classifiedState=unclassified&fromchange=b50418331e042dc2e82a59bf0384d5b6eada886a&selectedJob=293788324
https://treeherder.mozilla.org/logviewer.html#/jobs?job_id=293788324&repo=autoland&lineNumber=2014

Should be fixed now, I hadn't realized that there were even grid tests in dom.

https://hg.mozilla.org/mozilla-central/rev/ac09f1b654eb
https://hg.mozilla.org/mozilla-central/rev/3af2a45f142f
https://hg.mozilla.org/mozilla-central/rev/7f70bf534b1f
https://hg.mozilla.org/mozilla-central/rev/8c1c830a9b51
https://hg.mozilla.org/mozilla-central/rev/3d7db672d69e
https://hg.mozilla.org/mozilla-central/rev/3d385f974d44
https://hg.mozilla.org/mozilla-central/rev/d52f097a39dc