346185 - fixing one of two misspelled words in a line makes spellcheck underline disappear on both

Status: RESOLVED FIXED

(Core :: Spelling checker, defect, P1)

Description

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

Fixing one of two misspelled words in a line makes the spellcheck underline disappear on both words:  but only until something else forces a repaint.

Steps to reproduce:
 1. load this bug report
 2. in the textarea, type "gren and bron "
 3. click on the "e" in "gren"
 4. type the additional "e"
 5. do something to trigger a window repaint (sometimes moving the mouse pointer out of and into the window does this, otherwise cover the window with another one)

Expected results:
 4. underline disappears on "green" but remains on "bron"

Actual results:
 4. underline disappears on both  [seems to disappear on any misspelled words in the same line]
 5. underline returns on "bron"

Tested in a Linux trunk build from a day or two ago.

Bug 345979 *could* be an instance of this problem, but it doesn't have clearly described "Actual Results".

Comment 1

[Brett Wilson]

I think I understand the problem.

There's a flag on all frames that have a selection on them. When this flag is set, the frame goes and asks the selection where the selection is on that frame. The selection is then painted. Bug 345099 changed a lot of how the selection does this lookup. It shouldn't have affected how this works, but it could have.

When we remove a range from the selection, the code uses a content iterator to remove the selected flag from the frames. The problem is if there is another range on the frame, the frame no longer knows to ask the selection controller to update itself.

If I remove the code that clears the bit, everything works properly, but then the selected bits never get cleared, slowing things down as you create and remove selections.

I'm going to try to find if there are other selections on the affected nodes first, and not clear the bit when there is.

Comment 2

[Brett Wilson]

Attachment: Patch

Comment 3

[Uri Bernstein]

What happens if the selection ends at a non-text leaf node (e.g. an <img> or a <br>)?
In this case, I think GetEndContainer will return the parent of that node, and you'll be processing all the children of the parent, which might be a lot more than what you actually need.

Other than that, just some trivial comments about comments:

>+//    We therefore find any ranges that intersect the same nodes as the node
>+//    being removed
I think you mean "the range being removed"

>+    // select the length of the text
...
>+    // select the number of children
"get" or "find out" would be clearer than "select" in this context, although I'm not sure whether these comments are necessary at all.

>+  // clear the selected bit from the removed node's frames
"removed range's frames"?

Comment 4

[Uri Bernstein]

(In reply to comment #3)
> What happens if the selection ends at a non-text leaf node

I meant "the range", not "the selection".

Comment 5

[Brett Wilson]

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

Comment 6

[Brett Wilson]

Attachment: New patch

I think you are right, I only want to select the entire node when it is a text node. For non-text nodes, using the given offsets and finding all ranges touching those nodes is sufficient.

Comment 7

[Uri Bernstein]

Comment on attachment 231248 [details] [diff] [review]
New patch

r=me.

Comment 8

[Brett Wilson]

Fixed on trunk, leaving open for branch checkin.

Comment 9

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

Comment on attachment 231248 [details] [diff] [review]
New patch

a=drivers. Please land this on the MOZILLA_1_8_BRANCH

Comment 10

[Brett Wilson]

Fixed on branch.

Comment 11

[Brett Wilson]

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

Comment 12

[Ben klein]

This problem is back with a slight twist. It works fine if the two errors are on the 1st line of the email text area. But if the errors occur on a subsequent line the error reappears. Type the line

ths si a tst - corrected the 1st word other two underlines stayed, corrected 2nd word, underline on 3rd word disappeared

test ths si a tst  - corrected 'ths' underline on other two lines disappeared. 

The disappearance of the underline changes dependent on where the line is types. 

Ben Klein

Comment 13

[Jorg K (CEST = GMT+2)]

This bug is already marked "Resolved".

The disappearing underlines are reported here in bug 1100966.