Commit c89463a5 authored by adele@apple.com's avatar adele@apple.com

REGRESSION(3.2.3 - 4.0.2): Message composing: when I undo a color change to...

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@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@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
parent ad4434a6
2009-10-29 Enrica Casucci <enrica@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@chromium.org>
Reviewed by Darin Adler.
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
<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>
2009-10-29 Enrica Casucci <enrica@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@chromium.org>
Reviewed by Darin Fisher.
......@@ -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
......@@ -42,6 +42,8 @@ private:
virtual void doApply();
virtual void doUnapply();
virtual void doReapply();
void executeApply();
RefPtr<Element> m_element1;
RefPtr<Element> m_element2;
......
......@@ -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
......@@ -44,6 +44,8 @@ private:
virtual void doApply();
virtual void doUnapply();
virtual void doReapply();
void executeApply();
RefPtr<Element> m_element;
RefPtr<HTMLElement> m_dummySpan;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment