Skip to content
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-ui-4][css-sizing-3] Who defines box-sizing? #2458

Closed
annevk opened this issue Mar 20, 2018 · 10 comments
Closed

[css-ui-4][css-sizing-3] Who defines box-sizing? #2458

annevk opened this issue Mar 20, 2018 · 10 comments

Comments

@annevk
Copy link
Member

annevk commented Mar 20, 2018

It seems both CSS Sizing and CSS UI put a claim on it. This makes linking it from other standards hard.

(In particular, I noticed this when looking at warnings for https://quirks.spec.whatwg.org/.)

@annevk annevk added css-sizing-3 Current Work css-ui-4 Current Work labels Mar 20, 2018
@Loirooriol
Copy link
Contributor

From #1906 it seems it should be CSS Sizing.

@annevk
Copy link
Member Author

annevk commented Mar 20, 2018

Thanks, it seems that was semi-fixed as nobody updated UI...

@fantasai
Copy link
Collaborator

@frivoal Mind removing from ui-4?

@fantasai fantasai removed the css-sizing-3 Current Work label Mar 20, 2018
@fantasai fantasai changed the title Who defines box-sizing? [css-ui-4] Who defines box-sizing? Mar 20, 2018
@frivoal
Copy link
Collaborator

frivoal commented Mar 21, 2018

@fantasai
I'd like to (since I was the one who reported #1906), but the the definition was rewritten (by you and/or Tab) when moving adding it to css-sizing, and I had not yet taken the time to check whether anything normative had been lost or changed during the rewrite.

Now I have, so here's my review:


The rewrite is generally an improvement, both in purely editorial terms, and because it makes us of more anchoring terminology than what was available for the css-ui version.

However:

  1. The statement that box-sizing affects flex-basis is good, but a new addition to the definition of box-sizing. I cannot find any test that supports it. There's a 2012 resolution about flexbox that supports it and it was in flexbox before, so that addition is correct, but adding tests when modifying post-CR normative prose, or at least flagging the need for it, would be good.

  2. Same comment about "Values affected by box-sizing include [...] those used in functional notations such as fit-content()". That addition is a good one, but it should be supported by tests, and currently isn't.

  3. The new examples are good, but you dropped the one that showed how box-sizing let you put two width: 50% things with a 1em border and have them fit side by side as their outer size adds up to 100%. I think that was worth having. Even if we've invented things like flexbox since that example was written, it remains true.

  4. The CSS-UI version defined terms line inner min width and monkey patched css2.1 to use these terms where appropriate rather than the ambiguous minimum width which might refer to the inner one or the outer one, as well as content width as opposed to just width. The resulting monkey patch was not pleasant to read (or to write), but CSS21 was judged sufficiently non obvious that @bzbarsky had requested (multiple times) full disambiguation, and that writing it (and the corresponding tests) uncovered bugs.
    You dropped both the term definitions and the money-patch, and replaced it with a much tighter statement that terms like width, minimum width and friends always refer to the inner size, defining them to be non-ambiguous.
    I don't think this is a good idea:

    • If you read the minutes leading to the the resolution recorded in [css-ui-4][css-sizing] Moving box-sizing to the right spec #1906, we were talking about moving the monkey-patch, not dropping it.
    • The monkey patch was needed because CSS2.1 does not consistently use width (and friends) to mean the inner size. It uses it to mean any of: the inner size, the width property, the specified/computer/used value of the width property. I agree that in the many cases, there's only one reasonable way to understand the terms, and it is fairly obvious which one it is. However, the fact that it 100% certain is what caused @bzbarsky to raise this issue in the first place, and the lack of interop discovered during testing further proves the point. Note that when this was raised, the spec already said that box-sizing affects [min-|max-]?[width|height], already said to subtract padding and border from the outer size to get to the inner size and to floor at 0 when box-sizing is border-box, and that browsers were already largely interoperable in the trivial cases.
    • Terms like width are inherently ambiguous, since they can easily in casual speech refer to various concepts. Just defining them in one of the possible meanings does not guarantee that it will be used correctly everywhere going forward, and we already know it has been used incorrectly in the past.
    • Since "the correct term" and "the ambiguous term" look identical, the reader of a spec cannot easily know if the spec author used the word in the precises sense or in the casual and ambiguous sense.

    Now, I don't like monkey patching more than anyone else, but I would recommend:

    • css-sizing (like css-ui-3 before it) should define anchoring terminology for the inner [min-|max-]?[with|height] that cannot accidentally be used when you mean something else. Since it already defines inner size, defining inner [min|max](imum)? size doesn't seem like much of a stretch. We can also use bikeshed's lt="" to make sure that we still link there if we use width height inline size or block size instead of just size
    • Spec authors may use convenient-but-amgbigous terms like "width" when context is sufficient to be sure about what is meant, but should use precise terminology when there's any chance of ambiguity.
    • Either include the monkey patch of CSS21 in css-sizing-3 (in an appendix if that makes you feel less dirty), or normatively point to the one in CSS-UI-3 until we complete the next point
    • Errata CSS21 to inline the patch (Since CSS-UI-3 is PR and should soon be REC we can normatively use the terminology from CSS2.1) OR write a precisely worded Level 3 alternative to CSS21/visudet

Point 1, 2, and 3 are not blocking, but for point 4, I think you'll need to convince me or I'll need to convince you, before dropping one spec for the other.

If you're ok with that, I'm happy to write a Pull Request that:

  • adds anchor terminology for inner [min|max](imum)? size with the relevant lt="" to css-sizing
  • puts the monkey-patch in an appendix
  • removes it box-sizing from css-ui-4
  • Optionally, adds back the example from css-ui along the ones in css-sizing.

If you're not OK with that, please say why :)


Historical references for point 4:

@frivoal
Copy link
Collaborator

frivoal commented Mar 28, 2018

Agenda+F2F in to resolve the disagreement. @fantasai and @tabatkins , if you end up agreeing with me or convincing me before the F2F, we can take it off.

@frivoal frivoal changed the title [css-ui-4] Who defines box-sizing? [css-ui-4][css-sizing-3] Who defines box-sizing? Mar 28, 2018
@fantasai
Copy link
Collaborator

fantasai commented Apr 9, 2018

I'm okay with normatively pointing at css-ui-3 and/or updating CSS2.1 to be more precise. Not OK with carrying around this monkey patch forever.

That said, the monkey patch can be compressed to a single sentence, since every unqualified reference in CSS2.1 resolves to the inner width/height: this is exactly what the monkey patch does, except it spends an entire awful page of English-description diffs to do so. If we have to keep it going forward, I want to keep it in compressed form.

I'm happy to give width/height a more ambiguous definition for L3 specs and add anchors for inner/outer min/max? size if that's seen as helpful.

@css-meeting-bot
Copy link
Member

The Working Group just discussed definition of box-sizing in css-sizing, and agreed to the following resolutions:

  • RESOLVED: Add an issue and a fix for 2.1 to disambiguate width for inner and outer width.
The full IRC log of that discussion <dael> Topic: definition of box-sizing in css-sizing
<dael> github: https://github.com//issues/2458
<dael> florian: Had resolution to move the definition to css sizing
<dael> florian: There was a large re-write while it happened. I filed this to disagree with some of it.
<dael> fantasai: We want to add a normative reference to UI 3. 2.1 refers to width and height but doesn't say if it's inner. UI 3 has a diff written in English to figure out how it works. It's an awful bit of spec writing we shouldn't add.
<dael> fantasai: Proposal is add a normative reference from CSS Sizing to UI 3. Also maybe fold the edits into 2.1 so it's not necessary to have this.
<dael> florian: And this monkey patch once written is mostly obvious, but when unwritten it's not clear.
<dael> florian: For referring to the monkey patch sounds good.
<dael> florian: Other part I raised is that you defined normatively width to mean inner-width and we haven't checked all our specs.
<dael> fantasai: In 2.1 any instance where it's ambig it's the inner width
<tantek> tantek: and test cases revealed bugs in the definition as well that we had to fix in css-3-ui
<dael> florian: Sometimes it means the value of the property.
<dael> fantasai: In those cases it's the value minus borders and padding. THat enture monkey patch it's the inner width. The fundimental concept of 2.1 is it's inner width. There might be cases outside of CSS 2 where we were less careful, but those are bugs in the spec.
<dael> florian: What I was thinking about is in practce in practical speech they're unavoidable. I'd rather normatively define and anchor term that's non-ambig so if you run into width you don't have to wonder if they meant innor or forgot to be careful
<dael> tantek: Aslo dimensial width vs [missed] It's not just inner vs outer, but also the width as a property in cascade. There's multi levels of ambig going on.
<dael> florian: To solve that I'd like the inner to be an anchor term which you can link the in bikeshed.
<dael> dbaron: Are the places where we say width and mean inline size?
<dael> florian: That too. In enough cases it's ambig but obvious enough and in those cases we should fix.
<tantek> s/Aslo dimensial/Also dimensional
<tantek> s/vs [missed]/vs property width, computed, cascaded etc.
<dael> dbaron: I think the fact that there is a second thing we ought to audit for maybe we should not assume here.
<dael> fantasai: CSS L3 I haven't looke dat position, but the rest are careful to use block size.
<dael> dbaron: But non-layout.
<dael> fantasai: Any spec TabAtkins or I worked on is being careful. Anyone else editing might want to look.
<dael> tantek: I think we're not focusing on the issue. I think we should resolve this that CSS UI defines box sizing. And then the plan moving forward is separate.
<dael> fantasai: Box sizing moved tot he sizing module. florian opened asking for the monkey path to be copied into box sizing. I think it's better to nromaive reference CSS UI from Sizing. Box sizing should have been in the sizing spec so it was in CSS UI.
<dael> gsnedders: CSS UI 3 have a normative reference?
<dael> tantek: No. It's fine in CSS3 UI and it's because form element behave that way. For external specs I'm not sure why we're talking double direction here.
<tantek> s/form element/form elements
<dael> florian: Prop is keep the box sizing definition out of UI 4 and into Sizing with a reference to the monkey patch in sizing and refer to sizing where it has better sizing. Mostly defined in sizing and refer to the monkey patch to CSS UI 3
<dael> florian: To be able to apply the monkey patch to 2.1...the other thing you dropped from UI to sizing is that i normatively defined width and the like.
<dael> fantasai: Box sizing has the terms defined but in pieces. Inner is separate from min/max size. But the terms to exist in sizing. I didn't feel like duplicating your version.
<dael> florian: I felt you underfined them.
<dael> fantasai: Inner is defined in sizing. In CSS 2.1 the spec is written witht he understanding that width means inner-width. If you want to read it with the context of width existing you need to know that. MOnkey patch can be compressed to a sentence saying it's referring to ineer-width/height. All your changes were about that.
<dael> fantasai: I think we should resolve on having 2.1 so that the potentially ambig references to width are corrected so we don't need awk patch.
<dael> tantek: We need to open an issue. This issue is multi spec.
<dael> fantasai: That's fine.
<gsnedders> I am strongly in favour of fixing 2.1 here.
<dael> Rossen: Let's take resolution to 2.1
<tantek> s/open an issue/open an issue on 21
<tantek> s/21/2.1
<dael> florian: Can we normatively reference the monkey patch from sizing?
<dael> fantasai: Yes
<fantasai> s/width existing/box-sizing existing/
<dael> Rossen: We need to update CSS 2.1 by normatively pointing UI 3?
<dael> fantasai: 2.1 to be edited.
<dael> tantek: That's why I'm suggesting a separate new issue.
<dael> Rossen: What's the 2.1 fix?
<fantasai> https://www.w3.org/TR/css-ui-3/#box-sizing
<dael> tantek: In florian's long comment on 2458. Errata CSS 2.1 bullet point.
<dael> florian: Trying to craft the wording isn't group. We should open the issue and until we fix 2.1
<dael> Rossen: Prop: Add an issue and a fix for 2.1 to disambiguate width for inner and outer width.
<dael> Rossen: Obj?
<dael> RESOLVED: Add an issue and a fix for 2.1 to disambiguate width for inner and outer width.

@frivoal
Copy link
Collaborator

frivoal commented Jun 6, 2018

PR #2739 should solve this issue. Review appreciated.

Also filed #2740 for the CSS2 part, as per the Berlin discussions.

@annevk
Copy link
Member Author

annevk commented Dec 11, 2018

Bikeshed still suggests:

spec:css-sizing-3; type:property; text:box-sizing
spec:css-ui-3; type:property; text:box-sizing

@tabatkins
Copy link
Member

speced/bikeshed@6dcb46b should fix that.

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

No branches or pull requests

6 participants