538466 - "ASSERTION: Wrong document somewhere" with contenteditable

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect)

Description

[Jesse Ruderman]

Attachment: testcase (browser must be foreground to trigger the bug)

Bug 537041 is fixed, but this new testcase also triggers the same assertion.

###!!! ASSERTION: Wrong document somewhere: 'GetCurrentDoc() == aPresShell->GetDocument()', file /Users/jruderman/central/content/base/src/nsGenericElement.cpp, line 400

Comment 1

[Boris Zbarsky [:bzbarsky]]

Well, the code in nsHTMLEditor::SelectAll just grabs the selection anchor node (which is in the textnode that's in the span that's in the paragraph that's in the body that's NOT in the document) and then calls GetSelectionRootContent() on it.  Then we assert.

I think we should probably just replace the assert in GetSelectionRootContent() with a check; fixing all the selection consumers who apparently feel that they can trust selections (when they actually can't) is too much hassle...

Masayuki, does that sound ok to you?  This is basically what bug 537041 comment 9 last sentence was about.

Comment 2

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

Did you think we should land nsGenericElement.cpp part of https://bugzilla.mozilla.org/attachment.cgi?id=419535&action=diff ?

If so, I think it's ok.

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes, that's exactly what I think.  r=me if you want to do that, or I can put up the patch and ask you for review.  ;)  Let me know.

Comment 4

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

ok, looks like the current tinderbox is safe for landing, I'll land the patch's part with r=bz.

Comment 5

[Masayuki Nakano [:masayuki] (he/him)(JST, +0900)(Got a cold so that using sick leaves)]

http://hg.mozilla.org/mozilla-central/rev/500e3dbc7d35

done.

Comment 6

[Boris Zbarsky [:bzbarsky]]

Thanks!