Bug 120726 - When deleting editable content, typing style should be reset when moving into another node.
Summary: When deleting editable content, typing style should be reset when moving into...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Arpita Bahuguna
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2013-09-04 23:18 PDT by Arpita Bahuguna
Modified: 2013-09-10 00:33 PDT (History)
6 users (show)

See Also:


Attachments
Testpage (368 bytes, text/html)
2013-09-04 23:19 PDT, Arpita Bahuguna
no flags Details
Patch (5.35 KB, patch)
2013-09-05 03:44 PDT, Arpita Bahuguna
no flags Details | Formatted Diff | Diff
Patch (5.95 KB, patch)
2013-09-06 02:16 PDT, Arpita Bahuguna
no flags Details | Formatted Diff | Diff
Patch for landing (5.96 KB, patch)
2013-09-09 23:57 PDT, Arpita Bahuguna
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Arpita Bahuguna 2013-09-04 23:18:43 PDT
1. Fetch the attached testpage.
2. Start back-deleting from the end of the word "World" which is in blue color.
3. Notice how after deleting a few characters of this word, typing anything new would take on the existing word's style, i.e. newly typed content would be in blue color.
4. However continue deleting and start deleting the word "Hello".
5. Notice that even this time new content will appear in blue color, instead of taking on the existing content's style.

Typing style should be reset or cleared when we move into a different node.
Comment 1 Arpita Bahuguna 2013-09-04 23:19:23 PDT
Created attachment 210551 [details]
Testpage
Comment 2 Ryosuke Niwa 2013-09-04 23:25:55 PDT
Confirmed. WebKit doesn't match either TextEdit or Firefox in this case.
Comment 3 Radar WebKit Bug Importer 2013-09-04 23:26:06 PDT
<rdar://problem/14916114>
Comment 4 Arpita Bahuguna 2013-09-05 03:44:13 PDT
Created attachment 210597 [details]
Patch
Comment 5 Ryosuke Niwa 2013-09-06 01:44:34 PDT
Comment on attachment 210597 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210597&action=review

> Source/WebCore/editing/DeleteSelectionCommand.cpp:283
>      // typing style at the start of the selection, nor is there a reason to 
>      // compute the style at the start of the selection after deletion (see the 
>      // early return in calculateTypingStyleAfterDelete).

Perhaps you should update this comment as well.

> LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:3
> +Dump of markup 1:

This is rather mysterious title.

> LayoutTests/editing/deleting/maintain-style-after-delete.html:19
> +Markup.dump(test);

You can pass in a description as the second argument. e.g. "Deleting the blue text and typing O immediately after it. O should be blue below"

> LayoutTests/editing/deleting/maintain-style-after-delete.html:24
> +Markup.dump(test);

e.g. "Deleting O and the preceding space, then typing W. W should not be in blue below"
Comment 6 Arpita Bahuguna 2013-09-06 02:16:19 PDT
Created attachment 210712 [details]
Patch
Comment 7 Arpita Bahuguna 2013-09-06 02:17:33 PDT
(In reply to comment #5)
> (From update of attachment 210597 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210597&action=review
> 
Many thanks for the review rniwa!

Have made the suggested changes and uploaded another patch.

> > Source/WebCore/editing/DeleteSelectionCommand.cpp:283
> >      // typing style at the start of the selection, nor is there a reason to 
> >      // compute the style at the start of the selection after deletion (see the 
> >      // early return in calculateTypingStyleAfterDelete).
> 
> Perhaps you should update this comment as well.
> 
> > LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:3
> > +Dump of markup 1:
> 
> This is rather mysterious title.
> 
> > LayoutTests/editing/deleting/maintain-style-after-delete.html:19
> > +Markup.dump(test);
> 
> You can pass in a description as the second argument. e.g. "Deleting the blue text and typing O immediately after it. O should be blue below"
> 
> > LayoutTests/editing/deleting/maintain-style-after-delete.html:24
> > +Markup.dump(test);
> 
> e.g. "Deleting O and the preceding space, then typing W. W should not be in blue below"
Comment 8 Ryosuke Niwa 2013-09-06 13:11:18 PDT
Comment on attachment 210712 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=210712&action=review

> LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:3
> +Deleting 'W' in blue color and then inserting 'O'. The following markup should show 'O' in blue color.:

We shouldn't have . before :.

> LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:9
> +Deleting the blue colored text and the preceding space and then inserting 'W'. 'W' should be not be in blue color in the following markup.:

Ditto.
Comment 9 Arpita Bahuguna 2013-09-09 23:57:09 PDT
Created attachment 211177 [details]
Patch for landing
Comment 10 WebKit Commit Bot 2013-09-10 00:26:19 PDT
Comment on attachment 211177 [details]
Patch for landing

Clearing flags on attachment: 211177

Committed r155425: <http://trac.webkit.org/changeset/155425>
Comment 11 WebKit Commit Bot 2013-09-10 00:26:22 PDT
All reviewed patches have been landed.  Closing bug.
Comment 12 Arpita Bahuguna 2013-09-10 00:33:48 PDT
(In reply to comment #8)
> (From update of attachment 210712 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=210712&action=review
> 
> > LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:3
> > +Deleting 'W' in blue color and then inserting 'O'. The following markup should show 'O' in blue color.:
> 
> We shouldn't have . before :.
> 
> > LayoutTests/editing/deleting/maintain-style-after-delete-expected.txt:9
> > +Deleting the blue colored text and the preceding space and then inserting 'W'. 'W' should be not be in blue color in the following markup.:
> 
> Ditto.

Thanks for the review rniwa! I made the aforementioned changes and landed the patch.