Closed Bug 1412110 Opened 7 years ago Closed 7 years ago

Assertion failure: !item->GetAbove(), at /builds/worker/workspace/build/src/layout/generic/nsFrame.cpp:3151

Categories

(Core :: Web Painting, defect)

58 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla58
Tracking Status
firefox-esr52 --- unaffected
firefox56 --- unaffected
firefox57 --- unaffected
firefox58 --- fixed

People

(Reporter: jkratzer, Assigned: mattwoodrow)

References

(Blocks 1 open bug)

Details

(Keywords: assertion, testcase)

Attachments

(5 files)

Attached file trigger.html
Testcase found while fuzzing mozilla-central rev d734e6acf777.
Flags: in-testsuite?
Attached file log_minidump.txt
The first testcase reloads repeatedly, and I think it crashes when I click it a certain way (without needing the reloads).

I reduced it quite a bit to this testcase, removing a lot of the complexity -- this just combines position:sticky, a list bullet, and contenteditable.
The failing assertion is brand-new, as of bug 1405146.  mattwoodrow, can you take a look?
Blocks: 1405146
If it helps, here's a log from a brief "rr" debugging session, including a backtrace and a framedump.

As you can see, the "item" in question is a nsDisplayStickyPosition, and its GetAbove() method returns a nsDisplayCaret* (which is non-null, hence the assertion failure).

(I assume the nsDisplayCaret is the blinking insertion-point caret from contenteditable, or something like that.)
Flags: needinfo?(matt.woodrow)
Component: Layout → Layout: Web Painting
(57 is unaffected, per comment 3)
Has Regression Range: --- → yes
We have code to manually invalidate the frame when we become/stop being the caret frame, so we don't need to worry about the transition from no wrapper -> wrapper.
Assignee: nobody → matt.woodrow
Flags: needinfo?(matt.woodrow)
Attachment #8924475 - Flags: review?(mikokm)
Comment on attachment 8924475 [details] [diff] [review]
Make sure we create a wraplist when we build a caret item

Review of attachment 8924475 [details] [diff] [review]:
-----------------------------------------------------------------

LGTM. It would be nice to have a comment to explain the logic behind canSkipWrapList and aCreatedContainerItem.

::: layout/generic/nsFrame.cpp
@@ +3505,5 @@
>        // return early.
>  
>        aBuilder->AdjustWindowDraggingRegion(child);
>        child->BuildDisplayList(aBuilder, aLists);
> +      if (aBuilder->DisplayCaret(child, aLists.Content())) {

This change seems unnecessary since we return immediately?

::: layout/painting/nsDisplayList.h
@@ +719,5 @@
>    bool IsCompositingCheap() const { return mIsCompositingCheap; }
>    /**
>     * Display the caret if needed.
>     */
> +  bool DisplayCaret(nsIFrame* aFrame, nsDisplayList* aList) {

Since you touch the line anyway, could you move the brace on a new line?
Attachment #8924475 - Flags: review?(mikokm) → review+
Blocks: 1414816
Pushed by mwoodrow@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7c87b4385113
Make sure we build a wrap list for the caret frame, since it will have multiple display items. r=miko
https://hg.mozilla.org/mozilla-central/rev/7c87b4385113
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla58
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: