Summary: | Deleting a paragraph of text should not add elements for typing style | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Ryosuke Niwa <rniwa> | ||||||||
Component: | New Bugs | Assignee: | Ryosuke Niwa <rniwa> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | darin, dglazkov, enrica, justin.garcia, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Ryosuke Niwa
2012-03-27 17:18:54 PDT
Created attachment 134181 [details]
Patch
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. 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"? 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 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
Created attachment 134354 [details]
Removed platform-specific results for 5994480-2.html
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. 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. Comment on attachment 134354 [details]
Removed platform-specific results for 5994480-2.html
Looks good to me.
Thanks for the review! Landing it now. Committed r112444: <http://trac.webkit.org/changeset/112444> |