344184 - Crash [@ FindNextNode] with this testcase in designMode

Status: RESOLVED FIXED

(Core :: Spelling checker, defect)

Description

[Martijn Wargers (dead)]

See upcoming testcase. When clicking inside the iframe of the testcase, current trunk builds of Mozilla crash.
I haven't been able to get it crash on current 1.8.1 branch builds.

Talkback ID: TB20804012G
FindNextNode   mozInlineSpellWordUtil::BuildSoftText   nsGlobalWindow::QueryInterface   gMidasParamTable

This regressed between 2006-07-03 and 2006-07-04, indicating a regression from bug 339066.

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Mozilla crashes when clicking inside of the iframe.
You need to view the testcase locally, because of the use of enhanced privileges.
(the testcase is doing some odd things)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Crashes at FindNextNode are the top crash for the 1.8 branch.  Not sure if they're this bug or not, though.

Comment 3

[Robert O'Callahan (:roc) (email my personal email if necessary)]

It looks like we're dealing with a corrupt DOM. We find a SELECT element in the tree which has a null parent.

This may have something to do with the fact that the testcase puts HTML content inside a <textarea>, which is pretty disturbing. If so, I suspect this is not the main cause of reported FindNextNode crashes.

Comment 4

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Okay, we're crashing because mozInlineSpellChecker is remembering the last caret position as being at the <select> element, which has been removed. We try to find the word it's a part of, which doesn't work because it's not in the DOM anymore.

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Attachment: fix

Check if the "old caret position" content node is still in the DOM. If it is not, then we can just stop.

I think a better way to fix this would be to avoid caching mCurrentSelectionAnchorNode, but that might require changes to nsIEditor APIs, so I'll go with this for now. Also it seems to me that spellchecking could be confused by changes due to DOM API calls. We need to dig into this when brettw's back.

This actually could be the cause of various crashes, it's not really related to funky HTML inside textareas.

Comment 6

[Martijn Wargers (dead)]

Robert, did you mean to ask review on the patch?

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 229210 [details] [diff] [review]
fix

errrr yeah

Comment 8

[Martijn Wargers (dead)]

*** Bug 344729 has been marked as a duplicate of this bug. ***

Comment 9

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Can someone check this in for me? I am about to board a plane.

Comment 10

[Martijn Wargers (dead)]

Tree is closed until at least 7/15/2006, 6pm, see http://weblogs.mozillazine.org/it/

Comment 11

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Alright, I guess I'll check this in myself now that I'm in CA and have Internet access :-)

Comment 12

[Martijn Wargers (dead)]

Checking in mozInlineSpellChecker.cpp;
/cvsroot/mozilla/extensions/spellcheck/src/mozInlineSpellChecker.cpp,v  <--  moz
InlineSpellChecker.cpp
new revision: 1.19; previous revision: 1.18
done
Checking in mozInlineSpellWordUtil.h;
/cvsroot/mozilla/extensions/spellcheck/src/mozInlineSpellWordUtil.h,v  <--  mozI
nlineSpellWordUtil.h
new revision: 1.2; previous revision: 1.1
done

I just checked the patch into trunk, since I heard of your laptop problems (and ispiked said you wouldn't mind).

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

hehe, thanks for that

Comment 14

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 229210 [details] [diff] [review]
fix

need this crash fix on branch

Comment 15

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 229210 [details] [diff] [review]
fix

a=dbaron on behalf of drivers.  Please check in to MOZILLA_1_8_BRANCH and mark fixed1.8.1 once you have done so.

Comment 16

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fixed on branch

Comment 17

[Tracy Walker [:tracy]]

verified with Windows 2.0b2 build of 20060821