<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[164770] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/164770">164770</a></dd>
<dt>Author</dt> <dd>rniwa@webkit.org</dd>
<dt>Date</dt> <dd>2014-02-26 18:23:43 -0800 (Wed, 26 Feb 2014)</dd>
</dl>
<h3>Log Message</h3>
<pre>Indenting an indented image element resulted in an extra indentation
https://bugs.webkit.org/show_bug.cgi?id=129201
Reviewed by Enrica Casucci.
Source/WebCore:
The bug was caused by endOfParagraph returning a position at the beginning of a block when the position
passed into the function was at the beginning of the block. Consider the following DOM:
<blockquote><img></blockquote>
When endOfParagraph is called on (blockquote, 0), the condition r->isBR() || isBlock(n) in endOfParagraph
matches immediately on startNode and it returns (blockquote, 0) again.
This resulted in moveParagraphWithClones invoked by indentIntoBlockquote to erroneously clone the inner
blockquote. Worked around this bug in ApplyBlockElementCommand::formatSelection by checking this specific
condition and moving the position to the end of the block. Unfortunately, a lot of existing code depends
on the current behavior of endOfParagraph so fixing the function itself was not possible.
There was another bug in indentIntoBlockquote to incorrectly insert a new blockquote into the existing
blockquote due to the code introduced in <a href="http://trac.webkit.org/projects/webkit/changeset/99594">r99594</a> to avoid inserting before the root editable element.
Since this happens only if outerBlock is the root editable element, which is nodeToSplitTo or an ancestor
of nodeToSplitTo, explicitly look for this condition.
Test: editing/execCommand/indent-img-twice.html
* editing/ApplyBlockElementCommand.cpp:
(WebCore::ApplyBlockElementCommand::formatSelection):
(WebCore::isNewLineAtPosition):
* editing/IndentOutdentCommand.cpp:
(WebCore::IndentOutdentCommand::indentIntoBlockquote):
* editing/VisibleUnits.cpp:
(WebCore::endOfParagraph): Added a FIXME.
LayoutTests:
Added a regression test.
* editing/execCommand/indent-img-twice-expected.txt: Added.
* editing/execCommand/indent-img-twice.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreeditingApplyBlockElementCommandcpp">trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingIndentOutdentCommandcpp">trunk/Source/WebCore/editing/IndentOutdentCommand.cpp</a></li>
<li><a href="#trunkSourceWebCoreeditingVisibleUnitscpp">trunk/Source/WebCore/editing/VisibleUnits.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestseditingexecCommandindentimgtwiceexpectedtxt">trunk/LayoutTests/editing/execCommand/indent-img-twice-expected.txt</a></li>
<li><a href="#trunkLayoutTestseditingexecCommandindentimgtwicehtml">trunk/LayoutTests/editing/execCommand/indent-img-twice.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (164769 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2014-02-27 02:16:43 UTC (rev 164769)
+++ trunk/LayoutTests/ChangeLog 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2014-02-26 Ryosuke Niwa <rniwa@webkit.org>
+
+ Indenting an indented image element resulted in an extra indentation
+ https://bugs.webkit.org/show_bug.cgi?id=129201
+
+ Reviewed by Enrica Casucci.
+
+ Added a regression test.
+
+ * editing/execCommand/indent-img-twice-expected.txt: Added.
+ * editing/execCommand/indent-img-twice.html: Added.
+
</ins><span class="cx"> 2014-02-26 Bem Jones-Bey <bjonesbe@adobe.com>
</span><span class="cx">
</span><span class="cx"> [CSS Shapes] inset and inset-rectangle trigger assert with replaced element and large percentage dimension
</span></span></pre></div>
<a id="trunkLayoutTestseditingexecCommandindentimgtwiceexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/editing/execCommand/indent-img-twice-expected.txt (0 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/editing/execCommand/indent-img-twice-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-img-twice-expected.txt 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -0,0 +1,23 @@
</span><ins>+Test indenting an image element twice.
+
+Initial state:
+| <img>
+| src="../resources/abe.png"
+
+After indenting once:
+| <blockquote>
+| style="margin: 0 0 0 40px; border: none; padding: 0px;"
+| <#selection-anchor>
+| <img>
+| src="../resources/abe.png"
+| <#selection-focus>
+
+After indenting again:
+| <blockquote>
+| style="margin: 0 0 0 40px; border: none; padding: 0px;"
+| <blockquote>
+| style="margin: 0 0 0 40px; border: none; padding: 0px;"
+| <#selection-anchor>
+| <img>
+| src="../resources/abe.png"
+| <#selection-focus>
</ins></span></pre></div>
<a id="trunkLayoutTestseditingexecCommandindentimgtwicehtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/editing/execCommand/indent-img-twice.html (0 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/editing/execCommand/indent-img-twice.html (rev 0)
+++ trunk/LayoutTests/editing/execCommand/indent-img-twice.html 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -0,0 +1,23 @@
</span><ins>+<!DOCTYPE html>
+<html>
+<body>
+<script src="../../resources/dump-as-markup.js"></script>
+<div id="editor" contenteditable><img src="../resources/abe.png"></div>
+<script>
+
+Markup.description('Test indenting an image element twice.');
+
+var editor = document.getElementById('editor');
+editor.focus();
+document.execCommand('SelectAll', false, null);
+Markup.dump(editor, 'Initial state');
+
+document.execCommand('Indent', false, null);
+Markup.dump(editor, 'After indenting once');
+
+document.execCommand('Indent', false, null);
+Markup.dump(editor, 'After indenting again');
+
+</script>
+</body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (164769 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2014-02-27 02:16:43 UTC (rev 164769)
+++ trunk/Source/WebCore/ChangeLog 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -1,3 +1,37 @@
</span><ins>+2014-02-26 Ryosuke Niwa <rniwa@webkit.org>
+
+ Indenting an indented image element resulted in an extra indentation
+ https://bugs.webkit.org/show_bug.cgi?id=129201
+
+ Reviewed by Enrica Casucci.
+
+ The bug was caused by endOfParagraph returning a position at the beginning of a block when the position
+ passed into the function was at the beginning of the block. Consider the following DOM:
+ <blockquote><img></blockquote>
+
+ When endOfParagraph is called on (blockquote, 0), the condition r->isBR() || isBlock(n) in endOfParagraph
+ matches immediately on startNode and it returns (blockquote, 0) again.
+
+ This resulted in moveParagraphWithClones invoked by indentIntoBlockquote to erroneously clone the inner
+ blockquote. Worked around this bug in ApplyBlockElementCommand::formatSelection by checking this specific
+ condition and moving the position to the end of the block. Unfortunately, a lot of existing code depends
+ on the current behavior of endOfParagraph so fixing the function itself was not possible.
+
+ There was another bug in indentIntoBlockquote to incorrectly insert a new blockquote into the existing
+ blockquote due to the code introduced in r99594 to avoid inserting before the root editable element.
+ Since this happens only if outerBlock is the root editable element, which is nodeToSplitTo or an ancestor
+ of nodeToSplitTo, explicitly look for this condition.
+
+ Test: editing/execCommand/indent-img-twice.html
+
+ * editing/ApplyBlockElementCommand.cpp:
+ (WebCore::ApplyBlockElementCommand::formatSelection):
+ (WebCore::isNewLineAtPosition):
+ * editing/IndentOutdentCommand.cpp:
+ (WebCore::IndentOutdentCommand::indentIntoBlockquote):
+ * editing/VisibleUnits.cpp:
+ (WebCore::endOfParagraph): Added a FIXME.
+
</ins><span class="cx"> 2014-02-26 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Fix two assertions/crashes in compositing code
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingApplyBlockElementCommandcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp (164769 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp 2014-02-27 02:16:43 UTC (rev 164769)
+++ trunk/Source/WebCore/editing/ApplyBlockElementCommand.cpp 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -126,6 +126,14 @@
</span><span class="cx"> rangeForParagraphSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
</span><span class="cx"> endOfCurrentParagraph = end;
</span><span class="cx">
</span><ins>+ // FIXME: endOfParagraph can errornously return a position at the beginning of a block element
+ // when the position passed into endOfParagraph is at the beginning of a block.
+ // Work around this bug here because too much of the existing code depends on the current behavior of endOfParagraph.
+ if (start == end && startOfBlock(start) != endOfBlock(start) && !isEndOfBlock(end) && start == startOfParagraph(endOfBlock(start))) {
+ endOfCurrentParagraph = endOfBlock(end);
+ end = endOfCurrentParagraph.deepEquivalent();
+ }
+
</ins><span class="cx"> Position afterEnd = end.next();
</span><span class="cx"> Node* enclosingCell = enclosingNodeOfType(start, &isTableCell);
</span><span class="cx"> VisiblePosition endOfNextParagraph = endOfNextParagrahSplittingTextNodesIfNeeded(endOfCurrentParagraph, start, end);
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingIndentOutdentCommandcpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/IndentOutdentCommand.cpp (164769 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/IndentOutdentCommand.cpp 2014-02-27 02:16:43 UTC (rev 164769)
+++ trunk/Source/WebCore/editing/IndentOutdentCommand.cpp 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -108,7 +108,7 @@
</span><span class="cx"> // Create a new blockquote and insert it as a child of the root editable element. We accomplish
</span><span class="cx"> // this by splitting all parents of the current paragraph up to that point.
</span><span class="cx"> targetBlockquote = createBlockElement();
</span><del>- if (outerBlock == start.containerNode())
</del><ins>+ if (outerBlock == nodeToSplitTo)
</ins><span class="cx"> insertNodeAt(targetBlockquote, start);
</span><span class="cx"> else
</span><span class="cx"> insertNodeBefore(targetBlockquote, outerBlock);
</span></span></pre></div>
<a id="trunkSourceWebCoreeditingVisibleUnitscpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/editing/VisibleUnits.cpp (164769 => 164770)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/editing/VisibleUnits.cpp 2014-02-27 02:16:43 UTC (rev 164769)
+++ trunk/Source/WebCore/editing/VisibleUnits.cpp 2014-02-27 02:23:43 UTC (rev 164770)
</span><span class="lines">@@ -1231,6 +1231,7 @@
</span><span class="cx"> continue;
</span><span class="cx"> }
</span><span class="cx">
</span><ins>+ // FIXME: This is wrong when startNode is a block. We should return a position after the block.
</ins><span class="cx"> if (r->isBR() || isBlock(n))
</span><span class="cx"> break;
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>