Bug 110979

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!

<rdar://problem/36070206>