Summary: | Selection direction is not preserved when applying styles | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Shezan Baig <shezbaig.wk> | ||||||
Component: | HTML Editing | Assignee: | Sukolsak Sakshuwong <sukolsak> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | buildbot, commit-queue, dglazkov, jfernandez, mifenton, rniwa, webkit-bug-importer, webkit.review.bot | ||||||
Priority: | P2 | Keywords: | InRadar | ||||||
Version: | 528+ (Nightly build) | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
Bug Depends on: | 114612 | ||||||||
Bug Blocks: | |||||||||
Attachments: |
|
Description
Shezan Baig
2013-02-27 07:12:41 PST
Created attachment 190525 [details]
Patch
Comment on attachment 190525 [details] Patch Attachment 190525 [details] did not pass chromium-ews (chromium-xvfb): Output: http://webkit-commit-queue.appspot.com/results/16668112 New failing tests: editing/style/block-style-005.html Comment on attachment 190525 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=190525&action=review > Source/WebCore/editing/ApplyStyleCommand.cpp:189 > + Position base = wasBaseFirst ? newStart : newEnd; > + Position extent = wasBaseFirst ? newEnd : newStart; I would have put this directly in the line below but okay. > LayoutTests/editing/style/preserve-selection-direction-expected.txt:7 > +PASS sel.anchorOffset is 0 > +PASS sel.focusOffset is 5 By just looking at this output, it's not clear why these numbers should be correct. > LayoutTests/editing/style/preserve-selection-direction.html:14 > + var sel = window.getSelection(); Please don't use abbreviations like sel. > LayoutTests/editing/style/preserve-selection-direction.html:16 > + // test forward selection direction I don't think this comment is useful. > LayoutTests/editing/style/preserve-selection-direction.html:19 > + sel.collapse(editable.firstChild, 3); > + sel.extend(editable.firstChild, 8); > + document.execCommand('foreColor', false, 'green'); I would have put all of this inside evalAndLog. > LayoutTests/editing/style/preserve-selection-direction.html:23 > + // reset and test backwards selection direction I don't think this comment is useful. > LayoutTests/editing/style/preserve-selection-direction.html:27 > + editable.innerHTML = "This is some sample text"; > + sel.collapse(editable.firstChild, 8); > + sel.extend(editable.firstChild, 3); > + document.execCommand('foreColor', false, 'green'); Ditto. Comment on attachment 190525 [details]
Patch
r=me with me that.
Thanks, I will incorporate your comments, and I'm also looking into the failing test: block-style-005.html Comment on attachment 190525 [details] Patch Attachment 190525 [details] did not pass mac-wk2-ews (mac-wk2): Output: http://webkit-commit-queue.appspot.com/results/16791116 New failing tests: editing/execCommand/remove-format-orphaned-list-item.html editing/execCommand/4920488.html editing/execCommand/4920742-1.html editing/style/apply-style-atomic.html fast/events/autoscroll-in-textarea.html canvas/philip/tests/2d.text.draw.fontface.notinpage.html fast/frames/sandboxed-iframe-scripting.html fast/history/visited-link-background-color.html editing/style/apple-style-editable-mix.html editing/execCommand/remove-format-background-color.html editing/execCommand/4786404-1.html editing/execCommand/5049671.html editing/style/remove-styled-element-with-style-span.html editing/execCommand/5573879.html editing/inserting/space-after-removeformat.html fast/events/event-sender-mouse-moved.html editing/execCommand/4786404-2.html editing/execCommand/remove-formatting.html http/tests/cookies/third-party-cookie-relaxing.html editing/style/style-text-node-without-editable-parent.html Comment on attachment 190525 [details] Patch Attachment 190525 [details] did not pass mac-ews (mac): Output: http://webkit-commit-queue.appspot.com/results/16855052 New failing tests: editing/execCommand/remove-format-orphaned-list-item.html editing/execCommand/4920488.html editing/execCommand/4920742-1.html editing/style/apply-style-atomic.html editing/style/apple-style-editable-mix.html platform/mac/editing/input/bold-node.html editing/execCommand/4786404-2.html editing/execCommand/remove-format-background-color.html editing/execCommand/4786404-1.html editing/execCommand/5049671.html editing/execCommand/5573879.html editing/inserting/space-after-removeformat.html editing/execCommand/remove-formatting.html editing/style/remove-styled-element-with-style-span.html editing/style/style-text-node-without-editable-parent.html Created attachment 197960 [details]
Patch
Comment on attachment 197960 [details] Patch Clearing flags on attachment: 197960 Committed r148378: <http://trac.webkit.org/changeset/148378> Thanks Sukolsak! |