1072758 - "Assertion failure: false (should have already reflowed the anonymous block child)"

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[Jesse Ruderman]

Attachment: testcase

Assertion failure: false (should have already reflowed the anonymous block child), at layout/svg/SVGTextFrame.cpp:432

This reminds me of bug 890783.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[violet.bugreport]

The cause is here: https://dxr.mozilla.org/mozilla-central/source/layout/generic/nsFrame.cpp#1062

For a non-display element like mask, we rely on the the DidSetComputedStyle() hook to notify the closest ancestor that is not a non-display to schedule a reflow. This works for normal style change, but doesn't work when the style changes to "display:none". Because then the frame is gone and won't receive a SetComputedStyle from RestyleManager, thus DidSetComputedStyle won't be called.

So we will miss the opportunity to schedule a reflow when the style changes to "display:none", leaving the anon block dirty.

Do you know how to hook the style change to "display:none"? If such a hook exists, we can easily fix this bug by calling ScheduleReflowSVGNonDisplayText there.

Comment 3

[Robert Longson [:longsonr]]

I don't know, you'd want to ask a style system expert like heycam (or dbaron).

Comment 4

[violet.bugreport]

Hi Emilio,

I have a question about the style/layout system. The situation is that when a frame has changed style, we need to notify some of its ancestor to schedule a reflow. nsIFrame::DidSetComputedStyle() works well in most of the case, but it doesn't work when style is changed to "display:none". Do you know how to get notified about a style change even if it changes to "display:none"?

For more context, please read the commend#2 in this thread.

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I guess the problem you're describing is also a problem when the <text> element is removed from the DOM, right?

The right way to do it is probably overriding DestroyFrom. Does that help?

Comment 6

[violet.bugreport]

(In reply to Emilio Cobos Álvarez (:emilio) from comment #5)

I guess the problem you're describing is also a problem when the <text> element is removed from the DOM, right?

The right way to do it is probably overriding DestroyFrom. Does that help?

Thanks for the pointers. The handling for removal is actually fine, after some debugging I find it has registered an MutationObserver to listen ContentRemoved event. So I think probably the bug can be fixed by listening on AttributeChanged, then check the new style, if it becomes "display:none" then to something.

Comment 7

[Emilio Cobos Álvarez (:emilio)]

No, that doesn't work, you can change the style by changing any attribute, and sometimes without attribute changes, like :hover.

Comment 8

[violet.bugreport]

Attachment: Bug 1072758 - ScheduleReflowSVGNonDisplayText when changing style to display none

DidSetComputedStyle won't be called if the style changes to "display:none".
To ensure the reflow is properly scheduled, we need to also hook DestroyFrom.

Comment 9

[Noemi Erli[:noemi_erli]]

Receiving this while attempting to land:
"We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmptn1a_g\npatching file layout/generic/nsFrame.cpp\nHunk #3 FAILED at 1050\n1 out of 3 hunks FAILED -- saving rejects to file layout/generic/nsFrame.cpp.rej\nabort: patch failed to apply', '')"

Comment 10

[violet.bugreport]

I've manually rebased the patch to current trunk.

Comment 11

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/2717903072b6
ScheduleReflowSVGNonDisplayText when changing style to display none r=emilio

Comment 12

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/2717903072b6