If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

Assertion failure: "Frame list should've been sorted in reflow" with flexbox, :after, table-cell

RESOLVED FIXED in mozilla24

Status

()

Core
Layout
--
critical
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Jesse Ruderman, Assigned: dholbert)

Tracking

(Blocks: 1 bug, {assertion, testcase})

Trunk
mozilla24
assertion, testcase
Points:
---
Dependency tree / graph
Bug Flags:
in-testsuite +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 2 obsolete attachments)

(Reporter)

Description

4 years ago
Created attachment 750578 [details]
testcase

Assertion failure: nsLayoutUtils::IsFrameListSorted<IsOrderLEQWithDOMFallback>(mFrames) (Frame list should've been sorted in reflow), at layout/generic/nsFlexContainerFrame.cpp:1111

Similar to bug 826483, which dholbert fixed two weeks ago.
(Reporter)

Comment 1

4 years ago
Created attachment 750579 [details]
stack (gdb)
(Assignee)

Comment 2

4 years ago
So, bug 867454 added a special-case for flex container's children that have ":after".

However, in this case, the :after frame isn't a *child* of the flex container -- it's wrapped inside several anonymous table boxes. (since it's a table-cell frame)

We should be able to use the existing logic in GetContentForComparison() to descend through those anonymous frames.
Assignee: nobody → dholbert
Status: NEW → ASSIGNED
Component: Canvas: 2D → Layout
Depends on: 867454
OS: Mac OS X → All
Hardware: x86_64 → All
(Assignee)

Comment 3

4 years ago
Created attachment 750744 [details] [diff] [review]
fix v1

This patch converts the existing "GetContentForComparison" helper-function into a more targeted "GetFirstNonAnonBoxDescendant" method, and calls it slightly earlier, so that we can benefit from it when checking for ::before and ::after.
Attachment #750744 - Flags: review?(bzbarsky)
(Assignee)

Comment 4

4 years ago
Created attachment 750747 [details] [diff] [review]
fix v1a [just tweaked a comment]

Oops; noticed a stale (& now duplicate) comment about << Same "order" value >>. 

I tweaked it to instead say "Usual case" (as opposed to the Special case above it, for ::before/::after)
Attachment #750744 - Attachment is obsolete: true
Attachment #750744 - Flags: review?(bzbarsky)
Attachment #750747 - Flags: review?(bzbarsky)
(Assignee)

Updated

4 years ago
Flags: in-testsuite?
Comment on attachment 750747 [details] [diff] [review]
fix v1a [just tweaked a comment]

Ah, very nice!  r=me
Attachment #750747 - Flags: review?(bzbarsky) → review+
(Assignee)

Comment 6

4 years ago
Thanks!

Try run, with the patches for this bug and Bug 825810:
https://tbpl.mozilla.org/?tree=Try&rev=21312c3ec51b
(Assignee)

Comment 7

4 years ago
Apparently I just eyeballed the reftest instead of actually running it, so it turns out it fails, but it fails in a way that's actually expected, so it's a test-bug and not a code-bug.

The test fails because we're ignoring "align-self" on the ::before/::after table-cell and table-row elements. This is correct behavior, because align-self only applies to flex items, and the cell/row aren't themselves flex items -- they get wrapped in tables, and the *tables* are the flex items.

So, I'm just going to rename the reftest to "flexbox-with-pseudo-elements-3.html" (instead of "-1b") and give it its own reference case.  (The reference case will be similar to flexbox-with-pseudo-elements-1-ref.html, but without the "align-self" settings.)

(Note: the reftest also happens to aborts in debug builds when the patch for bug 825810 is also applied, as it was in that Try run -- but that's an issue in that bug's patch & it has a relatively straightforward solution -- I'll discuss that abort issue over on bug 825810.)
(Assignee)

Comment 8

4 years ago
Created attachment 751410 [details] [diff] [review]
fix v1b [fixed reftest] [r=bz]

Here's the patch with that fixed.  Carrying forward r=bz.

Try run with that fixed (just reftests, & just this patch -- not bug 825810 anymore):  https://tbpl.mozilla.org/?tree=Try&rev=8a071ff5e5b4
Attachment #750747 - Attachment is obsolete: true
Attachment #751410 - Flags: review+
(Assignee)

Comment 9

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/56e10e157323
(Assignee)

Updated

4 years ago
Flags: in-testsuite? → in-testsuite+
https://hg.mozilla.org/mozilla-central/rev/56e10e157323
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in before you can comment on or make changes to this bug.