Bug 82401 – Deleting a paragraph of text should not add elements for typing style

• Component: New Bugs
• Status: RESOLVED FIXED

[Ryosuke Niwa]

Deleting a paragraph of text should reset typing style

[Ryosuke Niwa]

Created attachment 134181 [details]
Patch

[Darin Adler]

Comment on attachment 134181 [details]
Patch

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

> Source/WebCore/ChangeLog:3
> +        Deleting a paragraph of text should reset typing style

I don’t understand this bug title. Deleting a paragraph does not “reset typing style” after this patch. You delete the paragraph and then type and the typed text has the correct typing style.

What this patch does is get rid of the “persistent typing style” that was here but wasn’t needed. That change seems fine. It’s the bug title I object to.

[Ryosuke Niwa]

yeah, the bug title is quite misleading as is. Can I just insert "persisttent" there? Or maybe "Deleting a paragraph of text should not leave style"?

[WebKit Review Bot]

Comment on attachment 134181 [details]
Patch

Attachment 134181 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/12147852

New failing tests:
editing/inserting/5994480-2.html

[WebKit Review Bot]

Created attachment 134193 [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: <class 'webkitpy.common.config.ports.ChromiumXVFBPort'>  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick

[Ryosuke Niwa]

Created attachment 134354 [details]
Removed platform-specific results for 5994480-2.html

[Enrica Casucci]

Comment on attachment 134354 [details]
Removed platform-specific results for 5994480-2.html

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

> Source/WebCore/ChangeLog:9
> +        So change our behavior to match TextEdit and Firefox.

"We are changing our behavior" sounds better to me,

> LayoutTests/editing/deleting/delete-and-cleanup-expected.txt:8
> +PASS confirmedMarkup is '<b><div style="border: solid red"><br></div></b>'

Why does <b> stay? In all the other cases the markup is completely gone.

> LayoutTests/editing/deleting/delete-and-cleanup.html:41
> +testDelete("div", "<div><b><div style=\"border: solid red\"><i>Hello</i></div></b></div>", "<b><div style=\"border: solid red\"><br></div></b>");

Ditto.

[Ryosuke Niwa]

Comment on attachment 134354 [details]
Removed platform-specific results for 5994480-2.html

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

>> LayoutTests/editing/deleting/delete-and-cleanup-expected.txt:8
>> +PASS confirmedMarkup is '<b><div style="border: solid red"><br></div></b>'
> 
> Why does <b> stay? In all the other cases the markup is completely gone.

That's because b happens to be outside of div, which is a block element. We try not to modify things outside of the enclosing block.
In the long term, we might be able to use ApplyStyleCommand's algorithm to push down styles here. The algorithm can handle these cases pretty well.

>> LayoutTests/editing/deleting/delete-and-cleanup.html:41
>> +testDelete("div", "<div><b><div style=\"border: solid red\"><i>Hello</i></div></b></div>", "<b><div style=\"border: solid red\"><br></div></b>");
> 
> Ditto.

Ditto.

[Enrica Casucci]

Comment on attachment 134354 [details]
Removed platform-specific results for 5994480-2.html

Looks good to me.

[Ryosuke Niwa]

Thanks for the review! Landing it now.

[Ryosuke Niwa]

Committed r112444: <http://trac.webkit.org/changeset/112444>