search

w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.52k stars 673 forks source link

[css-values] The lh and rlh units are more complicated than what they seem. #3257

Closed emilio closed 2 years ago

emilio commented 6 years ago

https://drafts.csswg.org/css-values/#lh says:

Equal to the computed value of the line-height property of the element on which it is used, converting normal to an absolute length by using only the metrics of the first available font.

But the computed value of line-height depends on other properties at least in some browsers, so this suddenly becomes really tricky.

For example, <select>'s line-height computed value for comboboxes changes in WebKit and Blink to normal if the select is themed (based on the -webkit-appearance) property:

https://crisal.io/tmp/select.html

I'm considering doing the same in Firefox in https://bugzilla.mozilla.org/show_bug.cgi?id=1501908.

In any case, the used value does definitely depend on theming of comboboxes, for example, across all browsers (see above).

How are these units supposed to work in this case? We definitely don't want values to depend on used values. We should probably specify how these adjustments work and make them work across browsers before anyone implements these units.

cc @lilles @FremyCompany @zcorpan

emilio commented 6 years ago

https://bug1501908.bmoattachments.org/attachment.cgi?id=9020530 is probably a more permanent link that the one above.

fantasai commented 6 years ago

@emilio Seems a bit odd that -webkit-appearance affects the computed value of line-height, I recall implementers (particularly Mozilla) arguing for minimal interference among properties for computed values. Is -webkit-appearance your only example or are there more? Also, what happens if an author sets line-height on the OPTION elements or OPTGROUP? Does it take effect or no?

MatsPalmgren commented 6 years ago

@fantasai We currently have option,optgroup { line-height: normal !important; } in our UA sheet. I'm considering changing that, but I need to reverse-engineer what other UAs do first...

We used to have the same on <select> and when I removed the !important part there it broke Twitter since Gecko didn't have any "treat it as normal for themed comboboxes" code in place...

emilio commented 6 years ago

I'm all in to less interaction between properties.

Though sometimes (in particular in this case, where we need to implement this interaction because of compat), it may be easier to do at computed value time rather than at used value time.

Seems like blink's code for this is:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_theme_default.cc?l=303&rcl=a18d95d1f23d08476ebe0bae8ce198a4c1e93427

There's a bunch of other font controls that affect line height, looking at that file, and to:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/core/layout/layout_theme.cc?l=243&rcl=1d2414e576dce6b3d55550002fd8f04c72e469d9

emilio commented 6 years ago

Note that differences between computed value time line-height and used-value-time line-height special-cases are indistinguishable as of now (in theory, see https://bugs.chromium.org/p/chromium/issues/detail?id=899489).

But it's something we need to have interop on before even considering implementing these units.

emilio commented 4 years ago

Another interesting case is text autosizing, which in some browsers like WebKit affects the computed line-height, and in others like Gecko or Blink affects the used line-height.

emilio commented 4 years ago

Though I guess that also affects font-size so there's a precedent on how they should behave there.

fantasai commented 3 years ago

@emilio So what do you want us to do here?

emilio commented 3 years ago

@fantasai either specifying and getting interop on when the computed value of line-height changes after-the-fact (as in the themed combobox case) and deal with it somehow in the spec, or specify that these are used value adjustments that don't affect these units (and get browsers to change that before implementing these units).

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed lh and rlh units and form control complications.

The full IRC log of that discussion <fantasai> Topic: lh and rlh units and form control complications
<fantasai> github: https://github.com/w3c/csswg-drafts/issues/3257
<fantasai> [emilio is fussing with webex problems]
<fantasai> emilio: Issue here is that some elements treat their line height at different times, some are computed value and some used value
<fantasai> emilio: and not interop
<fantasai> emilio: so line height units wouldn't be interop across browsers
<fantasai> emilio: can't quite change the adjustments
<fantasai> emilio: e.g. single-line input elements act as normal if line-height is less than ... some tricky things
<fantasai> emilio: roughly interoperable in terms of effect
<fantasai> emilio: but we should agree, but if we don't agree on when to do the adjustment, the units will behave differently depending on whether computed-value time or used value time
<fantasai> Rossen_: do we have any consensus on what should be done?
<fantasai> iank_: Remind me, emilio?
<fantasai> iank_: Can you remind me, these adjustments are all to do with 'appearance'?
<fantasai> emilio: not really
<fantasai> emilio: at least chrome applies regardless of appearance
<fantasai> iank_: interesting
<fantasai> emilio: I can check but pretty sure it's not tied to appearance, at least with input
<fantasai> emilio: unsure about <select>
<smfr> q+
<fantasai> Rossen_: Do we already have experiments and consensus, or trying to establish now?
<fantasai> smfr: What if we just remove the units? is there sufficient demand for these units that we need to solve this problem?
<Rossen_> ack smfr
<Rossen_> ack fantasai
<fantasai> fantasai: Units were added to allow authors to spec margins and sizes in units of line height, to maintain vertical rhythm
<fantasai> fantasai: I don't think it makes sense to remove them because we can't figure out what to do with form controls, where they're unlikely to be used anyway
<fantasai> fantasai: It's better to resolve them to something random on form controls than it is to remove the units
<fantasai> Rossen_: What do you propose we do?
<fantasai> emilio: I think <select> does change computed line-height based on appearance, and input used line-height regardless of appearance, and [something something nested elements]
<iank_> our control elements are complicated wrt. to this as emilio suggests.
<fantasai> emilio: The resolution I want is that we have an interoperable definition, before we ship the units
<fantasai> Rossen_: Are you going to be the first ones shipping the units?
<fantasai> emilio: I think i filed this when WebKit added them behind a flag
<fantasai> emilio: I just noticed and looked into the weird edge cases
<fantasai> emilio: I think WebKit is the only one with an implementation right now
<fantasai> smfr: we turned it off because of this problem
<fantasai> Rossen_: so you would be OK to not ship them for awhile?
<fantasai> smfr: I think that's fine
<emilio> scribenick: emilio
<emilio> fantasai: I don't want to see these units get stuck on this edge case
<emilio> ... If we're hang up on this issue I'd be happy to define that on form control it resolves to 16px or something
<emilio> ... I don't think the value of these on a form control matters at all
<emilio> ... if that's what we're hang up on let's resolve
<miriam> +1 fantasai
<emilio> ... but I wouldn't like for this feature to get stuck on that, nobody is going to use it on form controls
<astearns> +1 to fantasai
<emilio> q+
<Rossen_> ack fantasai
<Rossen_> ack emilio
<fantasai> emilio: To be clear, I don't think this is a hard problem to solve. Just need to figure out what implementations are doing
<fantasai> emilio: and then decide whether we are fine with these units behaving oddly on <select>, because computed value would change after the fact
<fantasai> emilio: so like you would resolve the units based on the author-specified line-height, when might be overridden later
<fantasai> emilio: I don't think this is an unsolveable problem
<fantasai> emilio: first check if we're all doing the same, and then decide if the weird behavior is desirable, and document it
<emilio> fantasai: Happy to just make it undefined on the spec on form controls, I don't think it matters on form controls
<emilio> ... I don't think it's a high priority to make it work there
<fantasai> Rossen_: is that something we can start with? Make it undefined on form controls, and then define it later?
<astearns> would rather we pick a behavior
<fantasai> emilio: I would much rather prefer it was interoperable
<Rossen_> ack fantasai
<fantasai> emilio: Undefining it is going to backfire
<astearns> test one browser and pick that value :)
<fantasai> Rossen_: It would be undefined on form controls, and interoperable everywhere else
<fantasai> emilio: Someone will use it on form controls, and start depending on its behavior
<fantasai> iank_: agree with emilio
<vmpstr> +1
<fantasai> Rossen_: Either we leave it as undefined on form controls, or we use some agreed-upon value, or take this back to the issue and work with implementers to figure out how this should work
<fantasai> Rossen_: which option do we want to take?
<fantasai> emilio: I'm happy to document how I think implementations are going to work here
<fantasai> emilio: I know what Gecko does, but checking what blink/webkit is doing
<fantasai> emilio: and see if we agree on it
<fantasai> emilio: if we do then, fine, if we don't agree, then we can decide what is the least painful way to interop
<fantasai> Rossen_: Ok, let's do that. Let's have you go back and write that up in the issue, and once you have it we'll have something concrete to talk about
<fantasai> Rossen_: and see if rest of engines can agree
<fantasai> Rossen_: does that work?
<fantasai> emilio: Sure
<fantasai> Rossen_: OK, sounds like we're going with the third solution.
<fantasai> Rossen_: Emilio, once you have a proposal, let's go from there
<fantasai> Rossen_: anything else on this issue?
emilio commented 2 years ago

Line height adjustments I'm aware of in Gecko:

I can look into other browsers in a bit.

css-meeting-bot commented 2 years ago

The CSS Working Group just discussed lh and rlh are complicated, and agreed to the following:

The full IRC log of that discussion <TabAtkins> Topic: lh and rlh are complicated
<TabAtkins> github: https://github.com/w3c/csswg-drafts/issues/3257
<TabAtkins> emilio: I don't think this is too controversial
<TabAtkins> emilio: fantasai added it to agenda
<TabAtkins> emilio: main issue is browsers have different line-height fixups
<TabAtkins> emilio: sometimes at used, sometimes at computed time
<TabAtkins> emilio: the ones at computed time should change the lh unit
<TabAtkins> emilio: but that's not possible, you've computed everything at that point
<TabAtkins> emilio: at very least, would like to either agree...
<TabAtkins> emilio: get interop on these adjustments
<TabAtkins> emilio: so they don't cause weird interop issues
<TabAtkins> emilio: the gecko adjustments are in the issue comment
<TabAtkins> emilio: we force a minimum line-height on inputs at used value time
<TabAtkins> emilio: for select we force line-height normal at computed-value time
<TabAtkins> emilio: i believe these adjustments are done in other browsers, maybe in subtly different ways
<iank_> we do them differently.
<TabAtkins> emilio: forgot to look into details
<TabAtkins> emilio: just don't know how differently
<TabAtkins> iank_: for some form controls we will look at the computed style and apply a bunch of fixups right at the end of computed style
<TabAtkins> iank_: so the two you're referring to will force it to line-height:initial
<iank_> https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/layout/layout_theme_default.cc;l=201-205
<fantasai> TabAtkins: I agree with fantasai, whatever we do here doesn't make any difference for real uses cases of lh/rlh. We will put anything you want in the spec, don't care.
<TabAtkins> emilio: agree, it would just be great ot ahve those fixups documented
<TabAtkins> fantasai: further than taht - i think having them documented is a nice-to-have. don't really think they need to be interop
<TabAtkins> fantasai: if people ship without interop, that's fine.
<TabAtkins> emilio: given they're already for webcompat...
<TabAtkins> fantasai: right, they're to make sure the line height doesn't shrink below usable for a form control
<TabAtkins> fantasai: nobody's gonna depend on that for a form control
<TabAtkins> emilio: this is the sort of thing that always bites us
<fantasai> s/for a form contro/for using lh or rlh on a form control/
<TabAtkins> emilio: I think this shoudln't be hard to get right
<astearns> ack dbaron
<TabAtkins> fantasai: I'm totally happy to get it right. don't want to delay shipping for 5 years for this one small detail that doesn't matter
<TabAtkins> dbaron: I think if people have the lh unit they might sometimes use them for the height of a form control
<TabAtkins> astearns: people will use the unit everywhere
<TabAtkins> astearns: so ian is saying chrome's fixups are different...
<TabAtkins> iank_: yeah ours are different than what emilio described
<TabAtkins> iank_: we just force, at least for select, reset it to initial line-height
<TabAtkins> iank_: we've been slowly trying to remove these custom style fixups, but very gradually
<TabAtkins> iank_: you can imagine a lot of this depends on if the form element has auto appearance
<TabAtkins> iank_: so this is the difficulty
<TabAtkins> iank_: if we didn't have appearance, all of these could be UA stylesheets
<TabAtkins> iank_: but we explicitly allow things to be overwritten if they don't have the appearance
<emilio> https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/text_control_inner_elements.cc;l=173-194;drc=7fe33e343d08dc8815b5ca48b96f76e98a3d67c7 is for text controls, https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/core/html/forms/menu_list_inner_element.cc;l=37-45;drc=04861b524177edda323dc6c37ad58e9bf1e85c12 is for select
<TabAtkins> emilio: think i found the chromium bits
<TabAtkins> emilio: seems like in chromium you apply the fixups not on the element, but on the inner shadow dom of the control
<TabAtkins> iank_: not 100% sure on that
<TabAtkins> iank_: thought we did some on the actual element
<TabAtkins> astearns: so emilio, are you discussing putting these into the spec based on gecko?
<TabAtkins> emilio: not necessarily, just making sure lh/rlh dont' change across browsers
<TabAtkins> emilio: it seems to me that implementing lh/rlh do end up with the wrong value, but wrong consistently
<TabAtkins> emilio: so if you did a line-height less than 1, you'd get that value for lh unit even if the final line-height is 'normal'
<astearns> q?
<TabAtkins> dbaron: i think the key question is whether the fixup changes what the lh does on itself, and on descendants
<iank_> https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10518
<TabAtkins> dbaron: and if it's easy for impls to come to agreement we can just spec that
<iank_> https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10519
<TabAtkins> emilio: right and even descendants might not be too important for input, bc replaced
<TabAtkins> florian: do any of these have descendants?
<TabAtkins> emilio: can set font-size on <option>, yeah
<TabAtkins> iank_: these two examples show what we do with <select>
<TabAtkins> iank_: first has line-height on <select>, and we log 'normal' - it resets to initial
<TabAtkins> iank_: In second I set appearance:none and it logs 5px, so it shows fixup is dependent on appearance
<TabAtkins> iank_: we do all of this as a fixup step on the computed style at the very end
<TabAtkins> iank_: so an easy impl for us of lh unit would be these fixups apply at the end, and don't interfere with lh unit
<TabAtkins> emilio: I agree that's the most reasonable thing to do
<TabAtkins> iank_: and a lot of people do set appearance:none if they want more styling control, and it'll work then
<TabAtkins> astearns: so do we have enough?
<TabAtkins> emilio: yeah seems reasonable, as long as we agree that fixups dont' change the meaning of the lh/rlh units
<TabAtkins> emilio: firefox behaviro is similar
<TabAtkins> emilio: on <select> we do post-style fixup
<TabAtkins> emilio: On webkit maybe the same, they do weird stuff on form controls
<TabAtkins> iank_: yeah we've diverged a lot
<fantasai> TabAtkins: so proposed resolution is that these fixups don't affect these units on form controls or descendants
<TabAtkins> emilio: descendants might get affected, since we do computed fixup
<TabAtkins> emilio: but on the element itself it's clear
<TabAtkins> iank_: I think today fo rus if we apply this fixup, and some inner form control has line-height:inherit, it'll get the fixed up value, but don't think that's important
<TabAtkins> dbaron: so if it affects descendants but not itself is interoperable, maybe specify it even tho it's weird
<TabAtkins> emilio: eh, we've got some todos to shift some computation to layout time rather than compute time
<TabAtkins> emilio: don't think it matters too much since it only applies to a few form controls
<TabAtkins> fantasai: so we can spec it doesn't affect the form control itslef, but undefined whethe rit affects descendants
<TabAtkins> emilio: yeah
<TabAtkins> iank_: fine with that
<fantasai> s/we can/can we/
<emilio> https://www.software.hixie.ch/utilities/js/live-dom-viewer/?saved=10520
<TabAtkins> RESOLVED: browser "fixups" of line height on form controls do *not* affect lh/rlh on the element itself. Effect of fixups on lh/rlh of descendants is explicitly undefined.
<emilio> ScribeNick: emilio