[Webkit-unassigned] [Bug 41423] spelling underline gets lost on backspace

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=41423


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #66691|review?                     |review-
               Flag|                            |




--- Comment #14 from Tony Chang <tony at chromium.org>  2010-09-07 14:35:22 PST ---
(From update of attachment 66691)
> +document.execCommand("Delete", false);
> +sel.modify("move", "right", "line");
> +shouldBe("textInputController.hasSpellingMarker(0, 6)", "1");

Can you add some more test cases?  I think the current test works without this patch because the cursor is on "Secand" so the word gets checked when you move the cursor off of it.  For example, you could also test ForwardDelete or test having multiple misspelled words on the second line.

> diff --git a/WebCore/editing/CompositeEditCommand.cpp b/WebCore/editing/CompositeEditCommand.cpp
>      setEndingSelection(VisibleSelection(start, end, DOWNSTREAM));
> +    document()->frame()->editor()->removeMarkersBeforeMovingParagrah(endingSelection());
>      deleteSelection(false, false, false, false);

Out of curiosity, what happens if you don't remove the markers?  What is the benefit of removing the markers?

> diff --git a/WebCore/editing/Editor.h b/WebCore/editing/Editor.h
> +    // Invoked from CompositeEditCommand::moveParagraphs()
> +    void removeMarkersBeforeMovingParagrah(const VisibleSelection&);
> +    void markMisspellingsAfterMovingParagrah(const VisibleSelection&);

Did you mean to rename these to what Ojan suggested?  r- because of this, but will be happy to r+ with the methods renamed.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.