[css-inline] A question for the procedure to compute the resolved value of "line-height"

[joonghunpark]

I have a question about the procedure to compute the used value of "line-height".
I'm currently making a change on blink in
https://chromium-review.googlesource.com/c/chromium/src/+/1522053/3.
This change is for "line-height" to return used value instead of "normal keyword"
as resolved value.

In case of WebKit, fontMetrics().lineSpacing() value is used instead of 'normal' as below.

int RenderStyle::computedLineHeight() const
{
    const Length& lh = lineHeight();
    // Negative value means the line height is not set. Use the font's built-in spacing.
    if (lh.isNegative())
        return fontMetrics().lineSpacing();
    if (lh.isPercentOrCalculated())
        return minimumValueForLength(lh, computedFontPixelSize());
    return clampTo<int>(lh.value());
}

And Firefox returns used value "px" instead of "normal" also.

But I'm not sure what is the exact procedure to compute the used value for "line-height",
because I coundn't find that in spec.

Is it ok to use font metircs linespacing value like WebKit, or is there a exact procedure to compute the used value?

[ewilligers]

Context:
https://drafts.csswg.org/css-inline-3/#propdef-line-height
https://www.w3.org/TR/CSS2/visudet.html#propdef-line-height

[frivoal]

The logic for line-height described in css2.1 is known to be buggy.

It has been quite thoroughly reviewed and fixed, and css2.2 was made reliable. There remained some unsolved questions forline-height: normal, but it is a lot less wrong than what came before it.

However, for unrelated reasons (insufficiently careful edits had been made throughout the spec, so various parts were wrong, and which was which had become intractable), all changes to css2.2 had to be reverted to css2.1. We still haven't been able to re-apply all the known-to-be good changes.

This means the better definition of line height is currently in no spec at all. We should fix that, but until that happens, I recommend reading form the PR that did fix it at some point: #1993, while keeping an eye for possible issues. I'm aware of at least these:

• 'line-height: normal' definition should reflect reality of determining based on fonts #1254 (This one is partly solved, so you may want to skip the first half of the issue, but as the comments after 'line-height: normal' definition should reflect reality of determining based on fonts #1254 (comment) show, there are still some unresolved points)
• [css-lists] Should list marker affects line height? #2418
• [css-inline] Did root inline boxes become useless? #1551

Based on all that, I am not sure that what you're trying to do is possible: unlike the other values of the property, line-height: normal does not result in single a height per element. Each text run may end up having a different height. To me, that means that returning 'normal' as a keyword makes more sense. Unless there's a compat constraint, I think CSSOM should be fixed accordingly (non-normal values should still be absolutized, but normal should stay as a keyword).

[joonghunpark]

I see. Then current blink behavior returning "normal" keyword instead of px value for "line-height" is reasonable I think. In that case, in https://drafts.csswg.org/cssom/#resolved-values, it seems that "line-height" should be excluded from the css property list of "used value is resolved value". Thank you for your help. 2019년 3월 20일 (수) 오후 12:57, Florian Rivoal <notifications@github.com>님이 작성:

…

The logic for line-height described in css2.1 is know to be buggy. It has been quite thoroughly reviewed and fixed, and css2.2 was made reliable. There remained some unsolved questions forline-height: normal, but it is a lot less wrong than what came before it. However, for unrelated reasons (insufficiently careful edits had been made throughout the spec, so various parts were wrong, and which was which had become intractable), all changes to css2.2 had to be reverted to css2.1. We still haven't been able to re-apply all the known-to-be good changes. This means the better definition of line height is currently in no spec at all. We should fix that, but until that happens, I recommend reading form the PR that did fix it at some point: #1993 <#1993>, while keeping an eye for possible issues. I'm aware of at least these: - #1254 <#1254> (This one is partly solved, so you may want to skip the first half of the issue, but as the comments after #1254 (comment) <#1254 (comment)> show, there are still some unresolved points) - #2418 <#2418> - #1551 <#1551> Based on all that, I am not sure that what you're trying to do is possible: unlike the other values of the property, line-height: normal does not result in single a height per element. Each text run may end up having a different height. To me, that means that returning 'normal' as a keyword makes more sense. Unless there's a compat constraint, I think CSSOM should be fixed accordingly (non-normal values should still be absolutized, but normal should stay as a keyword). — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#3749 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACmEJmGO3W5IDa61c9sHB_DBD3epNy88ks5vYbGsgaJpZM4b9b3c> .

[frivoal]

Agenda+ to check if we should change CSSOM to say that the resolved value of line-height: normal is normal

[dbaron]

I filed bug 1536871 to change this behavior in Gecko.

[css-meeting-bot]

The CSS Working Group just discussed #3749.

The full IRC log of that discussion

<dael> Topic: #3749
<dael> github: https://github.com//issues/3749
<dael> dbaron: Isn't this talking about value for GCS which is another term?
<dael> florian: Resolved value, you're right
<dael> florian: This is raised by chromium b/c cssom spec says resolved value is used value and they were trying to find a way to return number of pixels but line-height:normal doesn't resolve into a single height
<AmeliaBR> s/#3749/The resolved value of "line-height"/
<dael> florian: line-height has 2 sets of values. All values other than normal resolve to a single height, normal resolves in different value per text run. You ask for gCS for element, not for text run so we can't retunr
<dael> dbaron: Chromium returns normal as a keyword?
<dael> florian: Yes
<dael> nigel: Reviewed this and was confused by spec with resolved value b/c line-height just takes you to css2 line ehight and doesn't way what resolved value is for line-height. Doens't seem spec says what it should be
<dael> florian: Kind of does depending on how you read it. For line-height it's in the sublist where it says resolved value is used value. It's unclear for line-height:normal that can be returned
<dael> nigel: I think you're expalining how to read the sublists. THat prob needs reformatting. Thank you
<dael> florian: line-height:normal can only be preserved as normal if we want something meaningful
<dael> nigel: You want to move line-height to computed value category?
<dael> florian: Not necessarily. There is another subgrouping of values for line-height: length and % are turned into absolute values at computed but numbers are numbers and computed. At used numbers become lengths. Do same thing but inherit differently
<dael> florian: Normal, regardless of computed or used time can't become a single number.
<dael> AmeliaBR: Are you asking us to say normal exists at used value time or are you asking to create a special cat to figure out resolve value for line-height being normal if computed value is normal?
<dael> florian: I think difference between those 2 is editorial b/c used can't be observed unless we say gCS returns it. In terms of what' observable it's same. I don't care how we phrase, just don't turn normal into a number
<dael> nigel: Happy to have a unitless number for line-height converted to a used value?
<dael> florian: Unitless stay as unitless at computed. At used value I suppose it's cat. that way b/c web compat requires it. That's usually why prop whose resolved is mapped to used value instead of computed mapping. I'm not trying to change values other than normal, want to avoid normal being cast to a length when it can't be
<dael> florian: Who is CSSOM editor?
<dael> florian: Emilio
<dael> Rossen_: Right
<dael> Rossen_: You can make a PR and emilio can merge
<dael> florian: Happy with a resolution we keep normal and work with emilio on phrasing
<dael> nigel: You talked about what is returned when it's normal. Do we have data on UAs for unitless numbers?
<dael> florian: I have not looked. I assumed spec wasn't wrong, but can look separately.
<dael> AmeliaBR: Working on a test case. Chrome does what florian is asking for
<dael> myles: We're talking about return on gCS?
<dael> florian: Yes
<dael> myles: That's scary, line-height has been around forever
<dael> florian: Changing spec to match chrome
<dael> myles: FF and webkit return pixel value
<dael> florian: Can you define what value you return?
<dael> myles: THis issue has a snippit of code that desc how we compute
<dael> florian: [looks at it]
<dael> florian: I'm not terrified about web compat given chrome does it already
<AmeliaBR> Test case: https://codepen.io/AmeliaBR/pen/bZmgqJ?editors=1011
<dael> myles: otoh FF and webkit don't return keyword
<dael> florian: You think this is kind of code websites could have specialized on to deal with browser difference?
<dael> Rossen_: I think the assertion made was line-ehight has been there forever and many sites depend on it. Predicting what will be broken or not is difficult. I agree Chrome market shre prob dictates some behavior on desktop, but not sure on mobile
<dael> florian: I wouldn't say chrome dictates behavior, but chrome doing it this way is strong evidence that doing it this was doesn't break the web
<dael> florian: And length value you would return doens't match layout so why return that?
<dael> fantasai: Also computed value is the normal keyword. We use used value where there's a compat reason to do so
<dael> myles: I won't object but we wouldn't be able to make this change w/o more research into how this would break
<dael> Rossen_: Can always defer to next week if you want to get some time and come back to use myles
<dael> florian: I guess...the question is what's alternative? Spec being wrong is bad. Chrome returning a length that's unrelated to actually used length seems weird
<dael> AmeliaBR: Is it really wrong? It's based on font of element. THe text spans inside might have different fonts but doesn't the parent element have a line height even if it's not used for anything?
<dael> florian: From the code snippit I don't know which font metrics we're talking about
<dael> myles: Primary
<dael> florian: Which metric of that font?
<dael> myles: Ascent+descent+line gap
<fantasai> AmeliaBR, the line height for normal is calculated per fragment, not per element
<dael> florian: I'm not good enough at fonts to phase this, but I believe in the past we discussed there isn't a single ascent or descent and the one we use aren't always the same
<dael> myles: FOr a particular font there are different values, but every browser picks one and uses it throughout. At least for the browsers I know
<dael> florian: You return line height of the strut?
<dael> florian: strut is the css2.1 term for the thing that keeps the line height fromcollaping to 0
<dael> myles: Sounds right to me
<dael> fantasai: line height used is calc per fragment of element so it's not nec the line height from first avail font. Can also include fallback
<dael> myles: Correct, also includes things like child elements
<dael> fantasai: THat's about height on line-box. THere's difference between line-box and fragment we're looking at. For an element when calc line-height which is what's used when calc height of linebox in most cases it's definite nmber, but varies by fragment with normal
<dael> fantasai: THe position of that length on the line is used in combination with other elements on the line to get the line box. We're trying to calc the line-height value that factors into line box calc and that varies by fragment if you have normal. You can't get that answer w/o doing layout
<dael> florian: I guess this boils down to that number from webkit isn't entirely meaningless, but if we're saying this is the used value it actually isn't
<dael> fantasai: Example: have line-height:normal and all text is using second font in list that's what would set the line height, but you'd returnt he first font. Given the intention of gCS is reach the computed value and we don't have a web compat reason to not it seems to me returning normal makes a lot of sense.
<dbaron> I think there's value in allowing the discussion to proceed a little further in the issue before bringing it to the call; might be more likely to take less than 20 minutes.
<dael> Rossen_: We're at the top of the hour. I don't want to force a resolution. What's captured is compelling enough for WK and FF team to noodle on this for a week and we'll put it in next week's agenda.
<dael> florian: Sounds reasonable.

[MatsPalmgren]

I don't really understand why the line-height property is singled out. Pretty much all of the properties where the resolved value = used value in this list may have different used values on each fragment. E.g. padding-top:10% on a block that is fragmented has 0px on all fragments except the first, unless ofc box-decoration-break:clone was specified, and if so there's no guarantee that the percentage basis is the same for all fragments so they may have a different used value in that case too.

IIRC, Gecko returns the used value for the first fragment for all these properties. It seems inconsistent to me to single out line-height as an exception when other properties have the same issue. For that reason it seems better that Blink adopts the WebKit/Gecko behavior. A pixel-value also seems more useful to authors than normal, even if the value can potentially be different in other fragments if they exist, but as I said, that's no different from padding/border/margin etc.

(Ideally, there should be an API for getting the values of all fragments, but that's a separate issue.)

[emilio]

Also, Gecko does have internal line-height values for form controls that right now are unobservable via the computed style (-moz-block-height) thanks to this property returning the used value. We'd need to figure out what to return for those, which is unfortunate.

[kojiishi]

Happy to discuss what we can do for better web compat. The context was that, @joonghunpark wanted to contribute the change to Blink, but we didn't understand what the desired behavior is, hence he came here to ask.

Do other browsers match to WebKit? I read @litherum's explanation, and also @joonghunpark showed me the WebKit code. If this is what other browsers do, this is a simple fix, and fast to compute. We can try to see if we can safely change. This is not a used value though, so it maybe nice to update the spec.

If other browsers do it differently, we may need to discuss what is the desired behavior. I wish it be simple and fast since we need to compute it for getComputedStyle regardless author needs it or not, but I don't have other strong opinions.

[kojiishi]

@MatsPalmgren

IIRC, Gecko returns the used value for the first fragment for all these properties. It seems inconsistent to me to single out line-height as an exception when other properties have the same issue.

Blink does that too, taking the first block fragment. This is the first case for Blink to handle inline fragment though, and there are more special cases to consider than other properties, so I'd like to understand what other browsers do.

[frivoal]

I don't really understand why the line-height property is singled out. Pretty much all of the properties where the resolved value = used value in this list may have different used values on each fragment

• We're not talking about fragments here, but about text runs, or even parts of text runs. This may be considered sorta similar if you want to, but no fragmentation (either in the multicol/pagination sense, or in the line break sense) needs to occur here; we just need font fallback to happen. We may want to deal with these chunks the same way we deal with Fragments. Or not. I'd lean towards not, because I don't think it is specified anywhere if and how we're supposed to slice text runs into multiple chunks based on font fallback (and/or other things?), so it's hard to talk about the height of the first one until we agree on what's supposed to be in the first one.

• Assuming I understand it correctly the cold snippet that was shared for webkit without looking up the functions it calls, and assuming it was described correctly, the behavior described as the webkit behavior does not give the height of the not the first chunk: If gives the height of a hypothetical chunk with text from the first available font. You may not have one of these, and even if you do, it may not be the first one. Let's say your font stack is "times new roman", "hiragino mincho", serif, that both times new roman and hiragino mincho are available, and that all the text in the element is in Japanese. The actual line height (of all text runs / fragments / chunks) will be based on hiragino mincho, but the getComputedStyle as described above would return the height of times new roman alone.

I think we could do a number of variants:

1. return normal as a keyword.

2. return the theoretical height based on the first available font, regardless of whether it is actually used.
   Possible nuance: the latest version of the spec text includes (based I believe on @dbaron saying that this was needed):

   User Agents may use different sets of font metrics when determining A and D
   depending on whether the 'line-height' property
   is 'normal' or some other value,
   for instance taking external leading metrics into account
   only in the 'normal' case.

   Since we're not returning something based on actual layout, which of the possible font metrics are we using? presumably the one that gets used for the normal case, but given that the code snippet in the first comment talks about "the metrics" of "the font" when there are multiple fonts each having multiple metrics, I am not sure this nuance is taken into account, so maybe not.

3. return the actual height of the first inline box (i.e. from the beginning until a line break, or the start of a different inline, whichever comes sooner), enclosing all glyphs actually used, going from the tallest ascender to the deepest descender, including the strut.

   Variants:

   1. if the element starts with an atomic/replaced inline, count the height of the first inline box after that
   2. if the element starts with an atomic/replaced inline, return the height of the strut, i.e. the theoretical height based on the first available font.

   Further nuances:

   • If the anchor point of a float or an abspos comes in the middle of that box, does that count a cutting that box in two or not?
   • If we have some other formatting changes, within the line (say, a color change), does that count as splitting the box or not? If we have <p>aあ<span style=color:red>अ</span>, and we're asking for the line height of the <p>, we we work with the union of the Latin and Japanese fonts only, or do we include the Devanagari one as well? How about if the span is unstyled?
   • If the element starts with a block level descendent, do we look at the first inline box after that, or to we go pretend there's an empty inline at the start, and therefore return the height of the strut?

4. return the actual height of the first non empty inline box chunk: based on the height of first glyph actually used (or equivalently, on the height of the first run of glyphs that all come form the same font).

(1) is what Blink does now. (2) Is what I believe webkit does now. If Firefox does something based "on the first fragment", I suppose it does one of the other variants, but I'm not sure which, or maybe it does (2) as well.

Doing anything other than (1) needs us to dig into the details of the various variants. Given that no use case has been brought up so far, and that Chrome shipping variant (1) is fairly strong evidence that this behavior is web compatible, I'd rather do that.