New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-align][css-multicol] Gutter properties computed value #2294

Closed
mrego opened this Issue Feb 8, 2018 · 7 comments

Comments

@mrego
Member

mrego commented Feb 8, 2018

These are the links to both specs:

In css-align it says:

Computed value: as specified, with <length>s made absolute, and normal computing to zero except on multi-column elements

In css-multicol it says:

Computed value: | as specified, with <length>s made absolute

I'm not sure if this difference is on purpose or not.
I understand that with the current text the computed value of column-gap: normal would be:

  • 0px if it's not a multicol container
  • normal if it's a multicol container

But now that we're generalizing the properties to apply in other layout models (like grid and flexbox), it seems weird that we have a difference there.
Currently getComputedStyle() is returning normal in all browsers but Edge (where it returns 0px).

I guess we could change this and we have 2 options:

  • We can say that it's the computed value for normal is normal.
  • Or that the computed value for normal is the used value.

Note that if the computed style of normal is normal, then that value won't be animatable. However if it's the used value (0px or 1em in multicol) then it'll be animatable.

[ Edited by @dbaron to fix <production>s that were getting interpreted as HTML tags. ]

@frivoal

This comment has been minimized.

Contributor

frivoal commented Feb 9, 2018

It seems to me that an author writing column-gap: inherit is not completely crazy. If you have a multicol inside a grid, or a grid inside a multicol, you might want to inherit the gap so that it's the same in both. In that case, you probably want normal to resolve to an absolute width, so that both the parent and the child can have the same gap.

Based on that, I suggest that normal should compute to a width always.

Your point about animation goes in the same direction.

This is a technically breaking change for multicol, but I have a very hard time thinking of situations where that would have any negative impact.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 9, 2018

@frivoal If you care that much about your gaps, you're probably specifying their sizes explicitly anyway.

I think it's fine to have 'normal' compute to 'normal'. Also it would be necessary to do this if we adopt #2285.

@fantasai fantasai added the Agenda+ label Feb 9, 2018

@frivoal

This comment has been minimized.

Contributor

frivoal commented Feb 10, 2018

If you care that much about your gaps, you're probably specifying their sizes explicitly anyway.

Right, probably. But then again, that argument only says why it is not necessary to make the change, not why it's a bad change.

Transitioning from whatever it is right now (which may be normal) to a specified length sounds useful too, and normal not computing to a length gets in the way of that.

'normal' computing to 'normal' would be necessary to do this if we adopt #2285.

I don't follow. Why? Normal could mean "get the value of the parent" in subgrid, while still computing a a value (absolutized length if it's a length, or percentage if that's what you get from the parent).

@MatsPalmgren

This comment has been minimized.

MatsPalmgren commented Feb 10, 2018

Having the computed value of one property depend on the computed values of other properties is problematic in general. In this case, it would depend on the computed values for column-width, column-count, display etc. Please also note that whether you actually get column boxes or not is decided during box construction and depends on more things than just computed values (it's not supported on most form control elements, etc). If I understand the spec correctly, these computed values would depend on the actual box type (which is decided after all computed values have already been resolved). While that can be implemented, it requires some ugly hacks and has performance penalties.

I'd prefer to make normal compute to normal.

@frivoal

This comment has been minimized.

Contributor

frivoal commented Feb 11, 2018

ah, yes. I wasn't looking at the full picture, but that made sense.

@fantasai

This comment has been minimized.

Contributor

fantasai commented Feb 12, 2018

@MatsPalmgren Since we don't use gaps for block containers that aren't multicol, we could make it just depend on the display value. But computing through doesn't help if we want to try for #2285 ; for that we need the keyword to compute to itself.

@css-meeting-bot

This comment has been minimized.

Member

css-meeting-bot commented Feb 28, 2018

The Working Group just discussed [css-align][css-multicol] Gutter properties computed value, and agreed to the following resolutions:

  • RESOLVED: column-gap: normal computed value returns normal
The full IRC log of that discussion <dael> Topic: [css-align][css-multicol] Gutter properties computed value
<dael> github: https://github.com//issues/2294
<dael> Rossen_: Summary is we have column gap normal value return as 0px for anything that's not a multi-column and 'normal' if it is a multicol
<dael> Rossen_: Issue raised by Igalia that get computed style where all browsers but edge return normal. Proposal was to make computed value to be normal everywhere.
<dael> Rossen_: The consensus on the issue seems to be normal computes as normal.
<dael> Rossen_: So I'm looking for feedback or other idea.
<dael> Rossen_: Should we just adopt proposed resolution or not change?
<dael> Rossen_: Does anyone care about this who is on the call?
<dael> tantek: Only thing I'd add is just +1 to Mats
<dael> Rossen_: What he's saying is common sense of not making style depend on layout and not make properties depend on other properties which is one of our guiding rules and I fully agree. Just curious on this issue.
<dael> rachelandrew: I agree with fantasai that people who care will specify anyway so if impl are happy to go with normal that makes sense.
<dael> Rossen_: I don't think we'll have a problem changing to normal. I think we're the only impl that returns 0. I'm assuming given all other impl return normal there shouldn't be a compat risk for us, or at least minimal.
<dael> tantek: Looking quick I didn't see a simple dom test to check current impl. I'd be curious to try it in other browsers to see if there is anything consensus-wise.
<dael> Rossen_: Right.
<dael> tantek: Can I request that before we resolve? To add a test to get information?
<dael> Rossen_: Absolutely. We don't have to resolve today. fantasai and florian are out. They and Sergio are active. I'm just looking for progress.
<dael> Rossen_: Sounds like we want examples and test cases to eval other impl.
<dael> tantek: At least to informt he decision better. SOunds like rough consensus but it's based on theoretical purity.
<dael> tantek: I'd like more concrete data.
<rego> http://w3c-test.org/css/css-align/gaps/column-gap-parsing-001.html
<dael> rego: THere's WPT tests for this. THat's why I realized Edge isn't returning normal and that's when I checked the specs.
<dael> tantek: Can you add a link to it?
<dael> rego: ^
<dael> tantek: Thanks.
<tantek> rego which of the tests?
<tantek> (of the 17)
<rego> the first one "Default column-gap is 'normal'"
<rego> in Edge it fails
<rego> I meant, it returns 0px
<dael> Rossen_: I see the same behavior as rego stated in the issue. In windows I see Edge report 0 and FF and Chrome return
<dael> Rossen_: 'normal'
<dael> tantek: There's 17 tests.
<dael> rego: First one is the one that fails
<dael> Rossen_: fwiw looking at IE I think we regressed this in edge unintentionally.
<dael> Rossen_: For the sake of argument this could be jsut a bug.
<dael> tantek: I see normal in FF & Chrome
<rego> and Safari
<dael> Rossen_: Yeah. That's what I said. Normal everywhere, FF, Chome IE 11. Edge is only one that does 0.
<dael> tantek: Safari also.
<dael> tantek: Okay. That makes it pretty clear.
<dael> Rossen_: Going back to the issue, is this something we want to punt or do we feel there is enough understanding to resolve?
<dael> tantek: I think there's enough understanding.
<dael> tantek: If fantasai or anyone else has new information we can follow our normal proceedure.
<dael> Rossen_: fantasai supports normal to normal in the issue.
<dael> Rossen_: Objections to resolving that column-gap: normal computed value returns normal?
<dael> RESOLVED: column-gap: normal computed value returns normal

@rachelandrew rachelandrew added this to Needs action in css-multicol-1 Mar 21, 2018

@frivoal frivoal removed the css-multicol-1 label Mar 29, 2018

@frivoal frivoal closed this in fbe335d Mar 29, 2018

@frivoal frivoal moved this from Needs action to Needs Testcase (WPT) in css-multicol-1 Mar 29, 2018

frivoal added a commit to frivoal/web-platform-tests that referenced this issue Mar 29, 2018

[css-aling] tests for row-gap / column-gap normal
Matching the w3c/csswg-drafts#2294 spec change

Not including tests for used value in multicol, because (so far) it is
UA defined.

@rachelandrew rachelandrew moved this from Needs Testcase (WPT) to Done in css-multicol-1 Apr 5, 2018

moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Apr 23, 2018

Bug 1398482 part 1 - [css-grid][css-flexbox][css-multicol] Add 'row-g…
…ap' and 'gap' properties; make 'grid-[column|row]-gap' and 'grid-gap' alias the respective unprefixed properties (Stylo part). r=emilio

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

mykmelez pushed a commit to mozilla/gecko that referenced this issue Apr 24, 2018

Bug 1398482 part 1 - [css-grid][css-flexbox][css-multicol] Add 'row-g…
…ap' and 'gap' properties; make 'grid-[column|row]-gap' and 'grid-gap' alias the respective unprefixed properties (Stylo part). r=emilio

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

emilio added a commit to emilio/servo that referenced this issue Apr 25, 2018

style: Add 'row-gap' and 'gap' properties; make 'grid-[column|row]-ga…
…p' and 'grid-gap' alias the respective unprefixed properties.

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

Bug: 1398482
Reviewed-by: emilio

emilio added a commit to emilio/servo that referenced this issue Apr 25, 2018

style: Add 'row-gap' and 'gap' properties; make 'grid-[column|row]-ga…
…p' and 'grid-gap' alias the respective unprefixed properties.

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

Bug: 1398482
Reviewed-by: emilio

emilio added a commit to emilio/servo that referenced this issue Apr 27, 2018

style: Add 'row-gap' and 'gap' properties; make 'grid-[column|row]-ga…
…p' and 'grid-gap' alias the respective unprefixed properties.

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

Bug: 1398482
Reviewed-by: emilio

emilio added a commit to emilio/servo that referenced this issue Apr 28, 2018

style: Add 'row-gap' and 'gap' properties; make 'grid-[column|row]-ga…
…p' and 'grid-gap' alias the respective unprefixed properties.

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

Bug: 1398482
Reviewed-by: emilio

fergald added a commit to fergald/csswg-drafts that referenced this issue May 7, 2018

Moggers added a commit to Moggers/servo that referenced this issue Jun 13, 2018

style: Add 'row-gap' and 'gap' properties; make 'grid-[column|row]-ga…
…p' and 'grid-gap' alias the respective unprefixed properties.

This also makes 'normal' the initial value for the grid-*
properties, per:
w3c/csswg-drafts#2294 (comment)

Bug: 1398482
Reviewed-by: emilio
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment