[SCM] WebKit Debian packaging branch, webkit-1.2, updated. upstream/1.1.90-6072-g9a69373
adele at apple.com
adele at apple.com
Wed Apr 7 23:15:33 UTC 2010
The following commit has been merged in the webkit-1.2 branch:
commit c89463a55db99bf8dad7fce7190798bcdf2a9ed3
Author: adele at apple.com <adele at apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date: Thu Oct 29 23:37:17 2009 +0000
REGRESSION(3.2.3 - 4.0.2): Message composing: when I undo a color change to text in Mail, undo/redo behaves strangely
<rdar://problem/7067033>
https://bugs.webkit.org/show_bug.cgi?id=30892
WebCore:
Patch by Enrica Casucci <enrica at apple.com> on 2009-10-29
Reviewed by Darin Adler.
This problem shows in any scenario where it is necessary to split a text
node to apply a style. SplitElementCommand and WrapContentsInDummySpanCommand both
have member variables initialized in the constructor to keep reference to elements
they need to operate upon. These reference are not updated when reapplying the command.
For this reason it is necessary to guarantee that unapply doesn not delete the references
and that these commands implement doReapply to correctly reuse the existing
elements.
Test: editing/undo/redo-style.html
* editing/SplitElementCommand.cpp:
(WebCore::SplitElementCommand::executeApply): Added.
(WebCore::SplitElementCommand::doApply): Modified to call executeApply.
(WebCore::SplitElementCommand::doUnapply): Doesn't release m_element1.
(WebCore::SplitElementCommand::doReapply): Added.
* editing/SplitElementCommand.h: Added doReapply and executeApply.
* editing/WrapContentsInDummySpanCommand.cpp:
(WebCore::WrapContentsInDummySpanCommand::executeApply): Added.
(WebCore::WrapContentsInDummySpanCommand::doApply): Modified to call executeApply.
(WebCore::WrapContentsInDummySpanCommand::doUnapply): Doesn't release m_dummySpan.
(WebCore::WrapContentsInDummySpanCommand::doReapply): Added.
* editing/WrapContentsInDummySpanCommand.h: Added doReapply and executeApply.
LayoutTests:
Patch by Enrica Casucci <enrica at apple.com> on 2009-10-29
* editing/undo/redo-style-expected.txt: Added.
* editing/undo/redo-style.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@50310 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index d88b955..9acfa39 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,12 @@
+2009-10-29 Enrica Casucci <enrica at apple.com>
+
+ REGRESSION(3.2.3 - 4.0.2): Message composing: when I undo a color change to text in Mail, undo/redo behaves strangely
+ <rdar://problem/7067033>
+ https://bugs.webkit.org/show_bug.cgi?id=30892
+
+ * editing/undo/redo-style-expected.txt: Added.
+ * editing/undo/redo-style.html: Added.
+
2009-10-29 Shinichiro Hamaji <hamaji at chromium.org>
Reviewed by Darin Adler.
diff --git a/LayoutTests/editing/undo/redo-style-expected.txt b/LayoutTests/editing/undo/redo-style-expected.txt
new file mode 100644
index 0000000..b09c578
--- /dev/null
+++ b/LayoutTests/editing/undo/redo-style-expected.txt
@@ -0,0 +1,51 @@
+EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > BODY > HTML > #document to 7 of DIV > BODY > HTML > #document
+EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > DIV > DIV > BODY > HTML > #document to 2 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 2 of #text > DIV > DIV > BODY > HTML > #document to 4 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 4 of #text > DIV > DIV > BODY > HTML > #document to 6 of #text > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: shouldChangeSelectedDOMRange:(null) toDOMRange:range from 0 of #text > SPAN > DIV > DIV > BODY > HTML > #document to 2 of #text > SPAN > DIV > DIV > BODY > HTML > #document affinity:NSSelectionAffinityDownstream stillSelecting:FALSE
+EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
+EDITING DELEGATE: webViewDidChange:WebViewDidChangeNotification
+This tests applying a background color at the beginning, the middle and the end of one sentence, then undo and redo.
+Bug 30892
+
+Hello!
+Hello!
+Hello!
+
+Test 1: PASSED
+Test 2: PASSED
+Test 3: PASSED
diff --git a/LayoutTests/editing/undo/redo-style.html b/LayoutTests/editing/undo/redo-style.html
new file mode 100644
index 0000000..b2095d3
--- /dev/null
+++ b/LayoutTests/editing/undo/redo-style.html
@@ -0,0 +1,45 @@
+<html>
+<body>
+This tests applying a background color at the beginning, the middle and the end of one sentence, then undo and redo.
+<p>
+<a href="https://bugs.webkit.org/show_bug.cgi?id=30892">Bug 30892</a>
+</p>
+<div contenteditable="true">
+<div id="test1">Hello!</div>
+<div id="test2">Hello!</div>
+<div id="test3">Hello!</div>
+</div>
+<br>
+<ul>
+<li>Test 1: <span id="c1"></span></li>
+<li>Test 2: <span id="c2"></span></li>
+<li>Test 3: <span id="c3"></span></li>
+</ul>
+
+<script type="text/javascript">
+function runTest(elem, node)
+{
+ var e = document.getElementById(node);
+ var before = e.innerHTML;
+ document.execCommand("hilitecolor", false, "#FF0000");
+ var after = e.innerHTML;
+ document.execCommand("undo");
+ var afterundo = e.innerHTML;
+ document.execCommand("redo");
+
+ document.getElementById(elem).appendChild(document.createTextNode(((before == afterundo) && (after == e.innerHTML))? "PASSED": "FAILED"));
+}
+
+if (window.layoutTestController) {
+ layoutTestController.dumpEditingCallbacks();
+ layoutTestController.dumpAsText();
+}
+
+var s = window.getSelection();
+s.setBaseAndExtent(document.getElementById('test1').firstChild, 0, document.getElementById('test1').firstChild, 2);
+runTest('c1', 'test1');
+s.setBaseAndExtent(document.getElementById('test2').firstChild, 2, document.getElementById('test2').firstChild, 4);
+runTest('c2', 'test2');
+s.setBaseAndExtent(document.getElementById('test3').firstChild, 4, document.getElementById('test3').firstChild, 6);
+runTest('c3', 'test3');
+</script>
diff --git a/WebCore/ChangeLog b/WebCore/ChangeLog
index 3843276..6b0e7b2 100644
--- a/WebCore/ChangeLog
+++ b/WebCore/ChangeLog
@@ -1,3 +1,34 @@
+2009-10-29 Enrica Casucci <enrica at apple.com>
+
+ Reviewed by Darin Adler.
+
+ REGRESSION(3.2.3 - 4.0.2): Message composing: when I undo a color change to text in Mail, undo/redo behaves strangely
+ <rdar://problem/7067033>
+ https://bugs.webkit.org/show_bug.cgi?id=30892
+
+ This problem shows in any scenario where it is necessary to split a text
+ node to apply a style. SplitElementCommand and WrapContentsInDummySpanCommand both
+ have member variables initialized in the constructor to keep reference to elements
+ they need to operate upon. These reference are not updated when reapplying the command.
+ For this reason it is necessary to guarantee that unapply doesn not delete the references
+ and that these commands implement doReapply to correctly reuse the existing
+ elements.
+
+ Test: editing/undo/redo-style.html
+
+ * editing/SplitElementCommand.cpp:
+ (WebCore::SplitElementCommand::executeApply): Added.
+ (WebCore::SplitElementCommand::doApply): Modified to call executeApply.
+ (WebCore::SplitElementCommand::doUnapply): Doesn't release m_element1.
+ (WebCore::SplitElementCommand::doReapply): Added.
+ * editing/SplitElementCommand.h: Added doReapply and executeApply.
+ * editing/WrapContentsInDummySpanCommand.cpp:
+ (WebCore::WrapContentsInDummySpanCommand::executeApply): Added.
+ (WebCore::WrapContentsInDummySpanCommand::doApply): Modified to call executeApply.
+ (WebCore::WrapContentsInDummySpanCommand::doUnapply): Doesn't release m_dummySpan.
+ (WebCore::WrapContentsInDummySpanCommand::doReapply): Added.
+ * editing/WrapContentsInDummySpanCommand.h: Added doReapply and executeApply.
+
2009-10-29 Jeremy Orlow <jorlow at chromium.org>
Reviewed by Darin Fisher.
diff --git a/WebCore/editing/SplitElementCommand.cpp b/WebCore/editing/SplitElementCommand.cpp
index 35dfc6f..37b725c 100644
--- a/WebCore/editing/SplitElementCommand.cpp
+++ b/WebCore/editing/SplitElementCommand.cpp
@@ -41,31 +41,35 @@ SplitElementCommand::SplitElementCommand(PassRefPtr<Element> element, PassRefPtr
ASSERT(m_atChild->parentNode() == m_element2);
}
-void SplitElementCommand::doApply()
+void SplitElementCommand::executeApply()
{
- RefPtr<Element> prefixElement = m_element2->cloneElementWithoutChildren();
-
if (m_atChild->parentNode() != m_element2)
return;
-
+
Vector<RefPtr<Node> > children;
for (Node* node = m_element2->firstChild(); node != m_atChild; node = node->nextSibling())
children.append(node);
-
+
ExceptionCode ec = 0;
-
+
Node* parent = m_element2->parentNode();
if (!parent)
return;
- parent->insertBefore(prefixElement.get(), m_element2.get(), ec);
+ parent->insertBefore(m_element1.get(), m_element2.get(), ec);
if (ec)
return;
- m_element1 = prefixElement.release();
-
+
size_t size = children.size();
for (size_t i = 0; i < size; ++i)
m_element1->appendChild(children[i], ec);
}
+
+void SplitElementCommand::doApply()
+{
+ m_element1 = m_element2->cloneElementWithoutChildren();
+
+ executeApply();
+}
void SplitElementCommand::doUnapply()
{
@@ -85,7 +89,14 @@ void SplitElementCommand::doUnapply()
m_element2->insertBefore(children[i].get(), refChild.get(), ec);
m_element1->remove(ec);
- m_element1 = 0;
}
+void SplitElementCommand::doReapply()
+{
+ if (!m_element1)
+ return;
+
+ executeApply();
+}
+
} // namespace WebCore
diff --git a/WebCore/editing/SplitElementCommand.h b/WebCore/editing/SplitElementCommand.h
index 2732762..7ea8f5b 100644
--- a/WebCore/editing/SplitElementCommand.h
+++ b/WebCore/editing/SplitElementCommand.h
@@ -42,6 +42,8 @@ private:
virtual void doApply();
virtual void doUnapply();
+ virtual void doReapply();
+ void executeApply();
RefPtr<Element> m_element1;
RefPtr<Element> m_element2;
diff --git a/WebCore/editing/WrapContentsInDummySpanCommand.cpp b/WebCore/editing/WrapContentsInDummySpanCommand.cpp
index 7622c1e..5320b69 100644
--- a/WebCore/editing/WrapContentsInDummySpanCommand.cpp
+++ b/WebCore/editing/WrapContentsInDummySpanCommand.cpp
@@ -38,35 +38,37 @@ WrapContentsInDummySpanCommand::WrapContentsInDummySpanCommand(PassRefPtr<Elemen
ASSERT(m_element);
}
-void WrapContentsInDummySpanCommand::doApply()
+void WrapContentsInDummySpanCommand::executeApply()
{
Vector<RefPtr<Node> > children;
for (Node* child = m_element->firstChild(); child; child = child->nextSibling())
children.append(child);
-
- RefPtr<HTMLElement> span = createStyleSpanElement(document());
-
+
ExceptionCode ec;
-
+
size_t size = children.size();
for (size_t i = 0; i < size; ++i)
- span->appendChild(children[i].release(), ec);
-
- m_element->appendChild(span.get(), ec);
-
- m_dummySpan = span.release();
+ m_dummySpan->appendChild(children[i].release(), ec);
+
+ m_element->appendChild(m_dummySpan.get(), ec);
}
+void WrapContentsInDummySpanCommand::doApply()
+{
+ m_dummySpan = createStyleSpanElement(document());
+
+ executeApply();
+}
+
void WrapContentsInDummySpanCommand::doUnapply()
{
ASSERT(m_element);
- RefPtr<HTMLElement> span = m_dummySpan.release();
- if (!span)
+ if (!m_dummySpan)
return;
Vector<RefPtr<Node> > children;
- for (Node* child = span->firstChild(); child; child = child->nextSibling())
+ for (Node* child = m_dummySpan->firstChild(); child; child = child->nextSibling())
children.append(child);
ExceptionCode ec;
@@ -75,7 +77,17 @@ void WrapContentsInDummySpanCommand::doUnapply()
for (size_t i = 0; i < size; ++i)
m_element->appendChild(children[i].release(), ec);
- span->remove(ec);
+ m_dummySpan->remove(ec);
}
+void WrapContentsInDummySpanCommand::doReapply()
+{
+ ASSERT(m_element);
+
+ if (!m_dummySpan)
+ return;
+
+ executeApply();
+}
+
} // namespace WebCore
diff --git a/WebCore/editing/WrapContentsInDummySpanCommand.h b/WebCore/editing/WrapContentsInDummySpanCommand.h
index b12131a..be3af66 100644
--- a/WebCore/editing/WrapContentsInDummySpanCommand.h
+++ b/WebCore/editing/WrapContentsInDummySpanCommand.h
@@ -44,6 +44,8 @@ private:
virtual void doApply();
virtual void doUnapply();
+ virtual void doReapply();
+ void executeApply();
RefPtr<Element> m_element;
RefPtr<HTMLElement> m_dummySpan;
--
WebKit Debian packaging
More information about the Pkg-webkit-commits
mailing list