836925 - "Assertion failure: clearedEntries == 0 (Map should be empty already)" after adopting parent of <bdi>

Status: RESOLVED WORKSFORME

(Core :: DOM: Core & HTML, defect)

Description

[Jesse Ruderman]

Attachment: testcase

[@ mozilla::nsTextNodeDirectionalityMap::RemoveElementFromMap]

Similar to bug 815276, which Simon fixed several weeks ago.

Comment 1

[Jesse Ruderman]

Attachment: stack

Comment 2

[Scoobidiver (away)]

On Windows: bp-bp-a4891018-63bf-47cd-abeb-84c492130131.

Comment 3

[Alex Keybl [:akeybl]]

Simon - please nominate for tracking on Firefox 20 if this is a critical issue.

Comment 4

[Simon Montagu :smontagu]

ehsan, did we fail to include adoptNode in the implementation plan for dirAuto? It seems we should be treating it the same as binding and unbinding nodes, but I may have got the wrong end of the stick.

Comment 5

[(no longer active)]

(In reply to Simon Montagu from comment #4)
> ehsan, did we fail to include adoptNode in the implementation plan for
> dirAuto?

Yeah, I think so.  Sorry about that.

> It seems we should be treating it the same as binding and unbinding
> nodes, but I may have got the wrong end of the stick.

That seems reasonable to me.

Comment 6

[Simon Montagu :smontagu]

Attachment: Patch

After more thought and debugging, I think comment 4 is wrong: the bug only exists with nodes that are not currently bound to the document tree, and the solution is to set the mTransfer flag when setting the properties associated with auto-direction.

There may still be problematic cases though - for example if you have something like
 <div dir=auto><span id = x>Hello</span></div>
and then do adoptNode(x)

More experimentation is probably needed before asking review, but feedback is welcome.

Comment 8

[Simon Montagu :smontagu]

This has morphed slightly since bug 861606

Comment 9

[Guilherme Lima]

I've hit this crash on http://letras.mus.br/ searching for a song and then loading the page with the lyrics. Report: https://crash-stats.mozilla.com/report/index/a903b6d3-9ab6-461e-ad58-14d592131201

Comment 10

[Simon Montagu :smontagu]

Comment 9 looks like a different crash to me, can you file a new bug?

Comment 11

[Pablo]

It seems this crash no longer occurs since I cant find any reports on it , I will close it as WFM and then if it starts to reoccur we can reopen the issue

Comment 12

[Pablo]

Reopening bug since there are crash reports in the last 6 months.

Comment 13

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee didn't login in Bugzilla in the last 7 months.
:hsinyi, could you have a look please?
For more information, please visit auto_nag documentation.

Comment 14

[Hsin-Yi Tsai (she/her) [:hsinyi]]

The assertion was removed. And there are no crash reports for the past 6 months. I think we should close this.