824297 - "ASSERTION: Wrong line container hint" with button, :first-letter, flex

Status: RESOLVED DUPLICATE of bug 812822

(Core :: Layout, defect)

Description

[Jesse Ruderman]

Attachment: testcase

###!!! 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

Comment 1

[Jesse Ruderman]

Attachment: stacks

Comment 2

[Daniel Holbert [:dholbert]]

If I replace "first-letter" w/ "first-line", I get a different assertion.  I filed bug 827168 on that.

Comment 3

[Daniel Holbert [:dholbert]]

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

Comment 4

[Boris Zbarsky [:bzbarsky]]

> 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.  :(

Comment 5

[Daniel Holbert [:dholbert]]

(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)

[1] http://lists.w3.org/Archives/Public/www-style/2013Jan/0053.html

Comment 6

[Daniel Holbert [:dholbert]]

(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>.)

Comment 7

[Boris Zbarsky [:bzbarsky]]

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.

Comment 8

[Daniel Holbert [:dholbert]]

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.