Focus on MathML in page causes crash

• Component: MathML
• Status: RESOLVED WORKSFORME

[Alex Milowski]

After waiting quite awhile for this page to render:

   https://eyeasme.com/Joe/MathML/MathML_browser_test

right click on any MathML content piece to attempt to get the web inspector and an
assertion fails:

ASSERTION FAILED: !renderer()->needsLayout()
(/Users/alex/workspace/WebKit/WebCore/dom/Node.cpp:793 virtual bool WebCore::Node::isFocusable() const)

I assume this means the MathML is still rendering somehow.

While the MathML should not take this long to render, I'm not certain that we should get a crash
from the focus check.

[Alex Milowski]

Created attachment 70920 [details]
Smallest Example of Crash

[Alex Milowski]

This crash is caused by an anchor wrapping the MathML.

[Alex Milowski]

After a bit of testing and debugging, the crash is coming from the anchor element (a) being marked for layout.  When the focus event happens, the crash asserts that the element shouldn't need layout.  I'm not sure why the anchor element needs layout but it is probably due to something from the child MathML element.

[Alex Milowski]

Digging a bit further I find that the inline renderer for anchor has m_normalChildNeedsLayout set to true but none of the children are marked as needing layout.

[Alex Milowski]

Bug 47781 may be related to this.  In that case, the MathML overflows the table cell so the table cell may also have unfinished layouts.

[François Sausset]

I just tried with r69946 and no crash occurs on the mentioned webpage or with the test case, even when using the Web Inspector.

[Alex Milowski]

(In reply to comment #6)
> I just tried with r69946 and no crash occurs on the mentioned webpage or with the test case, even when using the Web Inspector.

The crash only happens in the debug build.

[Darin Adler]

This “crash” is an assertion. Assertions are in debug builds only. Some assertions detect theoretical inconsistencies that are not always practical problems.

One possibility is that there is a different approach to layout here, and we just have to teach the assertion about the special objects that will always think they need layout.

Another possibility is that we can make some simple adjustment so these objects will be marked as not needing layout.

[Alex Milowski]

(In reply to comment #8)
> This “crash” is an assertion. Assertions are in debug builds only. Some assertions detect theoretical inconsistencies that are not always practical problems.
> 
> One possibility is that there is a different approach to layout here, and we just have to teach the assertion about the special objects that will always think they need layout.
> 
> Another possibility is that we can make some simple adjustment so these objects will be marked as not needing layout.

OK.  That makes sense.  Somehow I thought assertions were always on.

Nevertheless, what doesn't make sense is all the children are marked as not needing layout but the parent thinks that a child needs layout.  That is, m_normalChildNeedsLayout is true.

If you change the wrapping anchor to a block (i.e. a { display: block }) in the test, everything works as expected.   There is something odd going on.

[Alex Milowski]

Created attachment 71074 [details]
Another example with a second non-mathml child in the anchor

Clicking on 'bingo' also crashs in debug builds.

[Alex Milowski]

As expected, if you remove the MathML math element, the assertion doesn't fail.  Somehow MathML rendering must be notifying its parent that it needs to relayout its children yet the layout is never scheduled.

[Alex Milowski]

I don't think this crash exists anymore.  I cannot reproduce it in the latest version.

What's the process for closing this?

[chris fleizach]

Moving to resolved based on Alex's comments