Bug 18835 - spelling underlines disappear when they shouldn't
Summary: spelling underlines disappear when they shouldn't
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 525.x (Safari 3.1)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Hironori Bono
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2008-05-01 14:08 PDT by Eric Seidel (no email)
Modified: 2009-01-28 16:51 PST (History)
1 user (show)

See Also:


Attachments
A Quick Fix (3.54 KB, patch)
2009-01-08 00:04 PST, Hironori Bono
justin.garcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Eric Seidel (no email) 2008-05-01 14:08:03 PDT
This isn't 100% reproducible, but I can usually get it to happen with the following steps:

1. click into a textarea (add comment field on this page), or editable html region (gmail)
2. type the following: The quick brown foxxx jumped over
3. hit the back arrow four times to move the cursor to the beginning of 'over'
4. hit return

The red squiggle under foxxx will disappear.

I haven't tried on mac, just windows.
Comment 1 Hironori Bono 2009-01-08 00:04:12 PST
Created attachment 26522 [details]
A Quick Fix

I accidentally noticed a cause of this bug while investigating another issue.
Somehow the InsertLineBreakCommand::doApply() function forgot copying markers while splitting a text node with a line-break key even though there is a CompositeEditCommand::splitTextNode() function, which splits a text node and copy markers.
I'm not sure it is correct to call the CompositeEditCommand::splitTextNode() function here, though.
Comment 2 Justin Garcia 2009-01-08 00:50:28 PST
Comment on attachment 26522 [details]
A Quick Fix

so, deleteTextFromNode removes spelling markers from text even if the deletion doesn't effect them?  we should document that problem.

do we need to make this change to InsertParagraphSeparatorCommand and BreakBlockquoteCommand?

r=me
Comment 3 Justin Garcia 2009-01-08 00:55:48 PST
> so, deleteTextFromNode removes spelling markers from text even if the deletion
> doesn't effect them?  we should document that problem.

Oops my mistake, I misread the code, I thought it deleted what was after the caret, not what was before.  Disregard that bit.
Comment 4 Hironori Bono 2009-01-08 20:28:32 PST
Thank you for your review and comments.

> do we need to make this change to InsertParagraphSeparatorCommand and
> BreakBlockquoteCommand?

As far as I read the InsertParagraphSeparatorCommand::doApply() and BreakBlockquoteCommand::doApply() functions, they already use the splitTextNode() function to split a text node. (This is a reason why my quick fix uses the splitTextNode() function.)

Regards,

Hironori Bono
Comment 5 Darin Fisher (:fishd, Google) 2009-01-28 11:38:37 PST
http://trac.webkit.org/changeset/40317
Comment 6 Darin Fisher (:fishd, Google) 2009-01-28 16:51:02 PST
Added missing layout test results:
http://trac.webkit.org/changeset/40341