Bug 110979 – Selection direction is not preserved when applying styles

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Shezan Baig]

ApplyStyleCommand::updateStartEnd does not preserve the original selection's direction

[Shezan Baig]

Created attachment 190525 [details]
Patch

[WebKit Review Bot]

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

[Ryosuke Niwa]

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.

[Ryosuke Niwa]

Comment on attachment 190525 [details]
Patch

r=me with me that.

[Shezan Baig]

Thanks, I will incorporate your comments, and I'm also looking into the failing test: block-style-005.html

[Build Bot]

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

[Build Bot]

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

[Sukolsak Sakshuwong]

Created attachment 197960 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 197960 [details]
Patch

Clearing flags on attachment: 197960

Committed r148378: <http://trac.webkit.org/changeset/148378>

[Shezan Baig]

Thanks Sukolsak!

[Radar WebKit Bug Importer]

<rdar://problem/36070206>