1222096 - XUL nsTextBoxFrame doesn't stop at inline-block boundaries when looking for text-decorations (which makes it hard to block propagation into <video> controls)

Status: NEW

(Core :: Layout: Text and Fonts, defect)

Description

[Daniel Holbert [:dholbert]]

STR:
 1. Load attatched testcase. (with nested inline-block elements, with the innermost one being a <video>)

 2. Look at the text-decorations.

ACTUAL RESULTS:
On the raw text, we only apply text-decorations up to the nearest "inline-block" parent. But for the text controls inside the <video>, we go all the way up to the root.

This seems to be because nsTextBoxFrame::DrawText walks all the way up the parent chain, looking for text decorations. It might need to stop when it finds an inline-block parent.

EXPECTED RESULTS:
The video should show (at most) the same level of text-decorations as the text alongside it.  (Really, we'd like it to show no text-decorations; that's bug 1218974.)

Comment 1

[Daniel Holbert [:dholbert]]

Attachment: testcase 1

Comment 2

[:Gijs (he/him)]

(In reply to Daniel Holbert [:dholbert] from bug 1218974 comment #13)
> Indeed, this is XUL's fault. It seems to be a bug in the way
> nsTextBoxFrame::DrawText walks up the tree looking for decorations.  The
> similar non-XUL code in nsTextFrame::GetTextDecorations walks up the tree &
> bails when it hits the first non-"display:inline" thing. But
> nsTextBoxFrame::DrawText doesn't bail out like that -- it looks all the way
> up the tree, through any number of block layers

So, AIUI, from bug 1777 comment 242 to 244, I suspect that this is because everything in XUL is -moz-box (barring explicit other decisions) and so if it really bailed at the first non-"display:inline*" thing then it would never inherit anything... which might be problematic?

(FWIW, I have no idea how to fix this, nor am I sure that my analysis makes sense, but that's what I came across when I looked for the implementation of the "stop at inline-block" thing in bugzilla, and it seems relevant...)

Comment 3

[Daniel Holbert [:dholbert]]

(I was actually slightly wrong in that comment you quoted, about 'first non-"display:inline" thing'.  From local testing, it looks like text-decorations *do* propagate through nested "display:block" elements -- not only through "display:inline".  They only stop when they hit an inline-block, probably because it's an atomic inline-level box or something like that.

So anyway, given that clarification: maybe the nsTextBoxFrame::DrawText() loop should stop when it hits the first "display:inline-block" ancestor? (or more generally, the first atomic inline-level ancestor)

Comment 4

[Daniel Holbert [:dholbert]]

(ni=dbaron for his thoughts, since he understands both text-decoration & XUL better than I do)

The code in question is here:
https://mxr.mozilla.org/mozilla-central/source/layout/xul/nsTextBoxFrame.cpp?rev=e8c7dfe727cd&mark=413-413,455-456#413
...which walks all the way up the tree, even through inline-block elements, grabbing all the decorations.  Vs. the 

For comparison, the equivalent non-XUL code is here:
https://mxr.mozilla.org/mozilla-central/source/layout/generic/nsTextFrame.cpp?rev=ec846e144024&mark=4903-4907,5001-5011,5030-5030#4903
...which has a "break" statement if we hit a "display:inline-block" element. (via a check for IsDisplayTypeInlineOutside & an explicit exclusion for "inline" itself.)

Comment 5

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I think I'd forgotten that code existed when making modifications to the nsTextFrame version.  If nsTextBoxFrame could reuse the nsTextFrame code that would be ideal... if not, porting recent fixes over would probably be good.

Based on the text at either:

  https://drafts.csswg.org/css2/text.html#decoration , which says:
  For all other elements it is propagated to any in-flow children.

or:

  https://drafts.csswg.org/css-text-decor/#line-decoration , which says:
  For all other box types, the decorations are propagated to all in-flow children.

I think there's not a problem with XUL boxes (and through display:flex, which should also fall into the "all other" case).  The decorations should, in fact, propagate through XUL boxes, and if the nsTextFrame code isn't producing that result, it should probably be fixed.

Comment 6

[Daniel Holbert [:dholbert]]

(In reply to David Baron [:dbaron] ⌚UTC+8 from comment #5)
> If nsTextBoxFrame could reuse the nsTextFrame code
> that would be ideal... if not, porting recent fixes over would probably be
> good.

Cool, thanks.

> I think there's not a problem with XUL boxes (and through display:flex,
> which should also fall into the "all other" case).  The decorations should,
> in fact, propagate through XUL boxes,

Agreed (and I think we depend on that working).

> and if the nsTextFrame code isn't
> producing that result, it should probably be fixed.

(Agreed, I think. Though, I don't know if the nsTextFrame is producing that result; that scenario (nsTextFrame-in-XUL) is sort of orthogonal to the cases considered here so far.)