Closed Bug 824297 Opened 7 years ago Closed 7 years ago

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

Categories

(Core :: Layout, defect)

x86_64
All
defect
Not set

Tracking

()

RESOLVED DUPLICATE of bug 812822

People

(Reporter: jruderman, Unassigned)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(2 files)

Attached file 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
Attached file stacks
If I replace "first-letter" w/ "first-line", I get a different assertion.  I filed bug 827168 on that.
OS: Mac OS X → All
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
Flags: in-testsuite?
> 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.  :(
Flags: needinfo?(bzbarsky)
(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
(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.
Flags: needinfo?(bzbarsky)
Depends on: 812822
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.