Bug 166813 - Unindenting text inside a blockquote can result in the text being reordered
Summary: Unindenting text inside a blockquote can result in the text being reordered
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Tim Horton
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2017-01-07 19:17 PST by Tim Horton
Modified: 2017-01-09 14:36 PST (History)
7 users (show)

See Also:


Attachments
Patch (4.80 KB, patch)
2017-01-07 19:18 PST, Tim Horton
no flags Details | Formatted Diff | Diff
Patch (11.31 KB, patch)
2017-01-08 00:26 PST, Tim Horton
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tim Horton 2017-01-07 19:17:49 PST
Unindenting text inside a blockquote can result in the text being reordered
Comment 1 Tim Horton 2017-01-07 19:18:11 PST
Created attachment 298298 [details]
Patch
Comment 2 Tim Horton 2017-01-07 19:18:52 PST
Hopefully someone who knows something about editing will tell me why this patch is wrong (and ... then ... how to fix it correctly).
Comment 3 Ryosuke Niwa 2017-01-07 19:35:39 PST
Comment on attachment 298298 [details]
Patch

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

r=me assuming two more test cases I suggested work as expected.

> LayoutTests/editing/execCommand/unindent-nested-blockquote-with-inner-div.html:15
> +if (window.testRunner)
> +    document.body.innerText = document.getElementById("description1").innerText + "\n" + edit1.innerHTML;
> +

Please use LayoutTests/resources/dump-as-markup.js
and dump markup before/after the outdent command.

Also, please add a test case for when second is inside a div as well as when it's inside a b/i/etc...
Comment 4 Ryosuke Niwa 2017-01-07 19:36:50 PST
Note it's very important for have those three test cases to be separate (can be in the same file).
Comment 5 Tim Horton 2017-01-08 00:26:47 PST
Created attachment 298299 [details]
Patch
Comment 6 Tim Horton 2017-01-08 00:27:37 PST
It looks reasonable to me; does this look OK to you, Ryosuke? And, thanks for pointing me to dump-as-markup! I seem to have gotten very unlucky with the test I decided to copy from :)
Comment 7 Ryosuke Niwa 2017-01-08 22:25:15 PST
Comment on attachment 298299 [details]
Patch

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

> LayoutTests/editing/execCommand/unindent-nested-blockquote-with-inner-div-expected.txt:65
> +|     <br>

This br needs to be inside the div.
Because br appears outside div, we're keeping blockquote after "second" in "after outdent".
Comment 8 Tim Horton 2017-01-09 14:26:19 PST
(In reply to comment #7)
> Comment on attachment 298299 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=298299&action=review
> 
> > LayoutTests/editing/execCommand/unindent-nested-blockquote-with-inner-div-expected.txt:65
> > +|     <br>
> 
> This br needs to be inside the div.
> Because br appears outside div, we're keeping blockquote after "second" in
> "after outdent".

Good call. Fixed, and everything still seems fine.
Comment 9 Tim Horton 2017-01-09 14:36:56 PST
https://trac.webkit.org/changeset/210524