345587 - Hang with spell-checked word in this testcase

Status: RESOLVED FIXED

(Core :: DOM: Editor, defect, P1)

Description

[Martijn Wargers (dead)]

See upcoming testcase.

To reproduce:
To reproduce:
-Right-click on the "tesxt" misspelled word (underneath this text) and choose a spell checked word

Result:
Mozilla hangs with 100% cpu

Expected result:
No hang and the word should have been spell checked

Comment 1

[Martijn Wargers (dead)]

Attachment: testcase

Comment 2

[Martijn Wargers (dead)]

I've extracted this testcase from google (http://www.google.com/webhp?hl=en&btnG=Google+Search )
- Set designmode to 'on' on the page
- Find in page 'froogle'
- Select 'Highlight All' in the find bar
- Now spell check the highlighted 'froogle' word

Comment 3

[Martijn Wargers (dead)]

This regressed on trunk between 2006-07-03 and 2006-07-04, very likely a regression from bug 339066.

Comment 4

[Brett Wilson]

It's weird, the editor is getting confused inserting text at this location:

#0  0x420d15de in nsRange::IsIncreasing (aStartN=0x0, aStartOffset=0,
    aEndN=0x8d593e4, aEndOffset=0)
    at /usr/local/google/home/brettw/branch/mozilla/content/base/src/nsRange.cpp:751
#1  0x420d04f0 in nsRangeUtils::ComparePoints (this=0x908a7a8,
    aParent1=0x8d59484, aOffset1=0, aParent2=0x8d593e4, aOffset2=0)
    at /usr/local/google/home/brettw/branch/mozilla/content/base/src/nsRange.cpp:337
#2  0x448ecfb4 in nsWSRunObject::GetWSPointAfter (this=0xbfffcdcc,
    aNode=0x8d59484, aOffset=0, outPoint=0xbfffcbec)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsWSRunObject.cpp:1942
#3  0x448ec2bd in nsWSRunObject::GetCharAfter (this=0xbfffcdcc,
    aNode=0x8d59484, aOffset=0, outPoint=0xbfffcbec)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsWSRunObject.cpp:1619
#4  0x448ededb in nsWSRunObject::CheckLeadingNBSP (this=0xbfffcdcc,
    aRun=0x8b6ffa0, aNode=0x8d59484, aOffset=0)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsWSRunObject.cpp:2197
#5  0x448e8767 in nsWSRunObject::InsertText (this=0xbfffcdcc,
    aStringToInsert=@0xbfffce1c, aInOutParent=0xbfffceec,
    aInOutOffset=0xbfffcdb8, aDoc=0x8a07e24)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsWSRunObject.cpp:324
#6  0x448ae5c3 in nsHTMLEditRules::WillInsertText (this=0x9136478,
---Type <return> to continue, or q <return> to quit---
    aAction=2000, aSelection=0x8a65aa0, aCancel=0xbfffce1c,
    aHandled=0xbfffcdcc, inString=0x87da6f8, outString=0xbfffd05c,
    aMaxLength=-1)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp:1467
#7  0x448ab844 in nsHTMLEditRules::WillDoAction (this=0x9136478,
    aSelection=0x8a65aa0, aInfo=0xbfffd01c, aCancel=0x1, aHandled=0x40212ba8)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/html/nsHTMLEditRules.cpp:589
#8  0x44903730 in nsPlaintextEditor::InsertText (this=0x8c1e7f8,
    aStringToInsert=@0x0)
    at /usr/local/google/home/brettw/branch/mozilla/editor/libeditor/text/nsPlaintextEditor.cpp:732
#9  0x449690cc in mozInlineSpellChecker::ReplaceWord (this=0x843b668,
    aNode=0x8d594d4, aOffset=3, newword=@0x87da6f8)

Comment 5

[Brett Wilson]

Attachment: Patch

The binary search algorithm in nsWSRunObject::GetWSPointBefiore and nsWSRunObject::GetWSPointAfter was plain wrong.

Comment 6

[Brian Ryner (not reading)]

Comment on attachment 232146 [details] [diff] [review]
Patch

Brett is making a new patch after we stepped through what the right behavior should be.

Comment 7

[Brett Wilson]

Attachment: Patch V2

Comment 8

[Brett Wilson]

Comment on attachment 232213 [details] [diff] [review]
Patch V2

Fixed on trunk, leaving open for branch checkin. Can we get this for beta 2?

Comment 9

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 232213 [details] [diff] [review]
Patch V2

a=drivers, go ahead and land this on the branch.

Comment 10

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

(Brendan wouldn't like the else-after-return, though, and removing the bogus unreachable returns might uncover some silly compiler warnings.)

Comment 11

[Brett Wilson]

Fixed on branch. I didn't change the if-else code (it should be changed on trunk if we really need to). This file already spits out more than a page of warnings, so even if we get another one on some compilers (I don't see one) it won't matter much :/