Closed Bug 824297 Opened 7 years ago Closed 7 years ago
"ASSERTION: Wrong line container hint" with button, :first-letter, flex
###!!! ASSERTION: NS_BLOCK_HAS_FIRST_LETTER_STYLE state out of sync: 'haveFirstLetterStyle == ((mState & NS_BLOCK_HAS_FIRST_LETTER_STYLE) != 0)', file layout/generic/nsBlockFrame.cpp, line 6441 (like in bug 399262) ###!!! ASSERTION: Wrong line container hint: '!aForFrame || (aLineContainer == FindLineContainer(aForFrame) || (aLineContainer->GetType() == nsGkAtoms::letterFrame && aLineContainer->IsFloating()))', file layout/generic/nsTextFrameThebes.cpp, line 1197
If I replace "first-letter" w/ "first-line", I get a different assertion. I filed bug 827168 on that.
OK, so it looks like this testcase basically hits the same problem as bug 812822 -- we've got a special HTML element (<button> in this case) which has a mandated frame-type (nsHTMLButtonControlFrame), so despite its display:flex styling, we don't end up generating a flex container frame for it -- BUT we **do** perform the "ApplyStyleFixups" flex-item-blockification to its children, and that violates its expectations. In this case, the child that we're blockifying wants to have "display: inline". (It's the label on the button, I think). That's not something we can't just whitelist like my current patch on bug 812822 does for "display: -moz-box"... We really do need to blockify display:inline stuff, in actual flex containers. So, I think we really need some way to make the ApplyStyleFixups tweak conditional on whether we're actually becoming a flex container or not. (which might mean it can't live in ApplyStyleFixups anymore). I'm not sure if there's a way to do that. For reference, that chunk of code (from bug 783415) is: https://mxr.mozilla.org/mozilla-central/source/layout/style/nsStyleContext.cpp?rev=18bc32f799d1&mark=358-402#358
Depends on: 783415
> I think we really need some way to make the ApplyStyleFixups tweak conditional on whether > we're actually becoming a flex container or not. More precisely based on whether our parent is. That's ... annoying. I need to think about this a bit. :(
(In reply to Daniel Holbert [:dholbert] from comment #3) > In this case, the child that we're blockifying wants to have "display: > inline". (It's the label on the button, I think). Actually, the child that we're blockifying _that triggers these assertions_ is the :first-letter pseudo-element. If I skip the blockification for :first-letter / :first-line, then the assertions in this bug & bug 827168 go away (aside from the "NS_BLOCK_HAS_FIRST_LETTER_STYLE state out of sync" assertion, which isn't flexbox-dependent & is tracked by bug 399262) That's good news, because per recent www-style discussion, we shouldn't be honoring :first-letter / :first-line on "display:flex" things anyway. (In this case our "display:flex" thing ends up being a button instead of a flex container... but I think it'd be fine if we ignored ":first-line" on it anyway)  http://lists.w3.org/Archives/Public/www-style/2013Jan/0053.html
(Note that we already seem to ignore :first-letter/:first-line on _actual_ flex containers -- e.g <div style="display:flex"> -- but we apparently try to honor them (& end up triggering these assertions after blockifying the resulting elements) when we specify a :first-[line|letter] style on elements w/ mandated frame-types like <buttons>.)
Whether first-line is allowed is controlled by nsCSSFrameConstructor::ProcessChildren's aAllowBlockStyles argument. For frames constructed by display type this is based on the display, but some things always pass true (because they sort of behave like inline-block on the inside). The things that always pass true are ConstructFieldSetFrame and anything with FCDATA_ALLOW_BLOCK_STYLES set in the fcdata (so legend, button; display:table-caption has that flag too, but that's display-based). So if all we need to worry about here is first-line and first-letter, then we can just check for the flex display types as needed in ConstructFieldSetFrame and in the ProcessChildren callsite in ConstructFrameFromItemInternal.
Verified that this bug's testcase doesn't fail the "Wrong line container hint" assertion anymore, now that bug 812822 has landed. (Now, the testcase *just* fires the assertion that's covered by bug 399262.) Marking as dupe of bug 812822, and marking in-testsuite+ because this bug's testcase was included in that bug's crashtests push.
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: in-testsuite? → in-testsuite+
Resolution: --- → DUPLICATE
Duplicate of bug: 812822
You need to log in before you can comment on or make changes to this bug.