Skip to content

[css-grid][cssom] Disallow repeat() syntax in grid-template-rows/columns resolved values #2427

Closed

Description

@MatsPalmgren

https://drafts.csswg.org/css-grid/#resolved-track-list

A contiguous run of two or more tracks that have the same size and associated line names may be serialized with the repeat() notation.

I tend to think that the above sentence should be removed from the spec and that the resolved values should always use the expanded form, i.e one value per track. UAs are currently returning incompatible values: Edge use repeat() syntax and other UAs don't. It's a burden for script to have to deal with two formats. I think one-value-per-track is far easier to work with so I propose we settle on that as the mandatory format.

Activity

  1. mrego commented on Mar 21, 2018

    @mrego
    Member

    Yes, I agree it'd be nice to just have one option here, and avoiding repeat() seems the simplest one.
    This will also help to simplify some of the WPT tests, as they need to check both options right now.

    I created a very simple example to show the issue: https://wptest.center/#/nsrrq1
    I guess we need @atanassov feedback about this, if Microsoft people are fine I guess the rest won't have any concern.

  2. FremyCompany commented on Mar 23, 2018

    @FremyCompany
    Contributor

    If Edge is actually the only browser using repeat then I guess it's only a matter of time before we get a compat case involving that. My initial reaction would be converge on the behavior of the majority in this case. I didn't think this through though, just a quick opinion.

  3. tabatkins commented on Mar 26, 2018

    @tabatkins
    Member

    Agenda+ because the editors don't care which way this goes, just tell us which way to edit it. ^_^

  4. atanassov commented on Mar 28, 2018

    @atanassov
    Contributor

    I'm not sure I quite agree with the argument here. Dealing with repeating syntax isn't new - see repeating gradients as an example. Expanding to a variable list that happened to fit a given content (in the case of auto placement) will give authors more trouble for parsing that if they deal with the short form.

    My preference would be for keeping the current text and open bugs for other impls to catch up.

  5. css-meeting-bot commented on Mar 28, 2018

    @css-meeting-bot
    Member

    The Working Group just discussed Disallow repeat() syntax in grid-template-rows/columns resolved values, and agreed to the following resolutions:

    • RESOLVED: serialization of the repeat() MUST use the repeat syntax
    The full IRC log of that discussion <dael> Topic: Disallow repeat() syntax in grid-template-rows/columns resolved values
    <dael> github: https://github.com//issues/2427
    <dael> Rossen_: Summary: there was a question about hey we are seeing a couple of ways repeat() is being serialized, why can't we have one?
    <dael> Rossen_: Edge supports serialization of repeat() by using repeat syntax and computed values inside.
    <dael> Rossen_: FF, Webkit, and Chrome serialize as a list of computed values.
    <dael> Rossen_: Question from TabAtkins and fantasai was just tell us which you want, we don't care.
    <leaverou_> What if the number to repeat by is a var()?
    <Rossen_> https://wptest.center/#/nsrrq1
    <dael> Rossen_: Seems Igalia is pushing for simpliest which is not use repeat() syntax and they asked MS to voice our thoughts. My position is having repeat() serialize as a list is more difficult for editors and script to handle. Test case inside the issue serialization is okay, but if the repeat >2 in chome if you use 5000 you get 100 columns and it drops. FF is between
    <fantasai> leaverou_, it's the used value that gets returned by gCS
    <leaverou_> Oh right
    <dael> Rossen_: Having to parse this many is harder. Finally, this isn't new. grandant-repeat() has the same challenge but we're not serializing a list.
    <dael> Rossen_: Our opinion is preserve the repeat syntax and have others catch up.
    <dael> Rossen_: Looking for other opinions.
    <dael> AmeliaBR: What if author has spec columns that could be collapse?
    <dael> AmeliaBR: If the author has literally specified multiple coluns without using repeat syntax but they could be collapsed using repeat would you serialize that using repleat?
    <dael> Rossen_: Not really. That's like saying if we have width 10px why not serialize as calc(10px). I don't believe shorter is the goal here.
    <dael> AmeliaBR: Just so long as there's a clear definition. You agrue keep as spec by author
    <dael> Rossen_: Right. Roundtrip of repeat serializes as repeat, not a bunch of values.
    <Rossen_> grid-template-columns: repeat(2000, 100px);
    <dael> Rossen_: In the test case from Igalia you have the repeat syntax that looks okay, but if you try this ^ the serialization is crazy.
    <dael> Rossen_: Any other opinions?
    <dael> Rossen_: If not, can we resolve?
    <dael> Rossen_: Objections to keeping repeat syntax as is currently in the spec and open bugs for other impl to do that?
    <dael> frremy: Spec is a may. So there is no implementation bug to file unless it's a must.
    <dael> Rossen_: A ha. good point frremy. I didn't realize this was a may.
    <dael> Rossen_: Objections to specifying the repeat() syntax serialization as a must?
    <AmeliaBR> +1 to interop!
    <dael> TabAtkins: Yes, we should spec one way or the other as a must.
    <dael> Rossen_: I'm not hearing objections.
    <dael> RESOLVED: serialization of the repeat() MUST use the repeat syntax
  6. fantasai commented on Mar 29, 2018

    @fantasai
    Collaborator

    OK, I didn't care which way this went, but the resolution above isn't valid. You can't serialize a specified value of repeat(5, auto) using repeat syntax, because we're serializing the used value of the track size and unless all your items are the same size, repeat(5, auto) isn't going to yield five tracks with identical pixel sizes that can be collaped as repeat(5, 25px) or whatever.

    We can have getComputedStyle use repeat() syntax according to some set of collapsing rules, but we can't make it mirror the structure of the specified style. Keeping in mind that we are serializing the used value here, the options here are:

    • Don't use repeat() in serialization. Easy, not nice for scaling up to giant grids.
    • Use repeat() in serialization to collapse any sequence of identically-sized columns. Simplest option for using repeat().
    • Use repeat() in serialization to collapse any sequence of identically-sized columns only if they happen to be sourced from the same specified repeat(). NOTE: A specified repeat() can be split into a sequence of multiple serialized repeat()s and/or externally-listed sizes if the repeated track list contains sizes that are not <length-percentage>. E.g. 20px repeat(6, auto) would serialize out as 20px repeat(2, 20px) 35px repeat(3, 19px) if that is what the track sizes end up as.
    • Some other option which is less aggressive than the previous, e.g. only collapse columns if they're in a repeat() that only contains <length-percentage> track listings.
  7. javifernandez commented on Mar 30, 2018

    @javifernandez
    Contributor
  8. mrego commented on Apr 2, 2018

    @mrego
    Member
    • Don't use repeat() in serialization. Easy, not nice for scaling up to giant grids.
    • Use repeat() in serialization to collapse any sequence of identically-sized columns. Simplest option for using repeat().

    I'd go for either one of these 2 options, I don't see the benefit of doing something more complex than that.

  9. FremyCompany commented on Apr 4, 2018

    @FremyCompany
    Contributor

    I would just note that in addition to the size difference, you also have to take care of the line names being the same for the duration of the repetition. Still siding on the don't use repeat side, because 3 browsers are already doing that.

  10. css-meeting-bot commented on Apr 4, 2018

    @css-meeting-bot
    Member

    The Working Group just discussed Disallow repeat() syntax in grid-template-rows/columns resolved values, and agreed to the following resolutions:

    • RESOLVED: Specify the current behavior in all the browsers except Edge. Just don't use repeat() in grid serialization
    The full IRC log of that discussion <dael_> Topic: Disallow repeat() syntax in grid-template-rows/columns resolved values
    <dael_> github: https://github.com//issues/2427
    <fantasai> ScribeNick: fantasai
    <fantasai> TabAtkins: So we resolved to serialize out the repeat()s that were specified,
    <fantasai> TabAtkins: but actually you can't do that as fantasai points out in https://github.com//issues/2427#issuecomment-377357237
    <fantasai> TabAtkins: So we need to choose some different option
    <fantasai> TabAtkins: There are a few possible options
    <fantasai> TabAtkins: First is to reverse previous resolution: don't use repeat() in serialization of gCS
    <fantasai> TabAtkins: This is very simple and straightforward
    <fantasai> TabAtkins: downside is it can potentially produce very long values for grid-template*
    <fantasai> TabAtkins: if you do something like grid-template-rows: repeat(10000, 1px)
    <fantasai> TabAtkins: Second option is to compress any adjacent tracks that have the same track size (and line names)
    <fantasai> astearns: Clarification on 2nd option, is that only when specified value was repeat() or anytime?
    <fantasai> TabAtkins: Anytime
    <fantasai> TabAtkins: more complicated variants are to track which tracks came from a repeat(), and then collapse them if possible
    <fantasai> (These options are summarized in https://github.com//issues/2427#issuecomment-377357237 )
    <fantasai> TabAtkins: The Igalia folks don't like tracking which things were in repeat() originally
    <fantasai> TabAtkins: seems like too much complexity for little gain
    <fantasai> TabAtkins: I think the best thing to do is to not serialize repeat(). The only issue is a blow-out of string sizes if you have long ones
    <fantasai> TabAtkins: but there is a cap on the number of tracks so it won't be too crazy
    <fantasai> florian: I think I'd go with non-collapsing things
    <fantasai> florian: Seems to me that there are a lot of corner cases
    <dael> florian: I think I'd go with non-collapsing things as well. As i'm looking through there seems to be lots of corner cases. I'm not sure they'll all be fine.
    <dael> florian: Given there's limited value le's skip the pain.
    <fantasai> ScribeNick: dael_
    <fantasai> ScribeNick: dae
    <fantasai> ScribeNick: dael
    <dael> florian: Given that you get things like calc that are somewhat the same. I'd rather just not go there.
    <dael> astearns: Only edge rep is melanierichards . Since Edge is the browser that retains repeat() and thought we should. I'd like to get an Edge opinion.
    <dael> fantasai: frremy says still siding on the don't use repeat() sidde
    <dael> TabAtkins: Rossen wants repeat() and frremt would rather not.
    <dael> astearns: Since frremy commented on the issue my Edge concern is satisfied.
    <dael> astearns: Other comments on if we should deal with repeats or throw them out?
    <dael> astearns: Prop: Specify the behavior in all the browsers except Edge. just don't use repeat() in grid serialization
    <dael> RESOLVED: Specify the current behavior in all the browsers except Edge. Just don't use repeat() in grid serialization
  11. mrego commented on Apr 23, 2018

    @mrego
    Member

    @atanassov please confirm you're fine with this as I'd like to update the tests accordingly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions