Title: [94497] trunk
- Revision
- 94497
- Author
- [email protected]
- Date
- 2011-09-03 16:30:08 -0700 (Sat, 03 Sep 2011)
Log Message
REGRESSION(r94274): selection-change-closes-typing.html fails
https://bugs.webkit.org/show_bug.cgi?id=67377
Reviewed by Kent Tamura.
Source/WebCore:
The problem was that when the shadow DOM is updated by setInnerTextValue, WebKit layer detects the selection
change and calls confirmCompositionWithoutDisturbingSelection, which in turn modifies the shadow DOM by
inserting text.
Fixed the bug by not inserting text in confirmCompositionWithoutDisturbingSelection. It turned out that this
function is only used to cancel composition but never to confirming composition and restoring selection.
Test: platform/mac/editing/input/selection-change-closes-typing-2.html
* editing/Editor.cpp:
(WebCore::Editor::confirmCompositionWithoutDisturbingSelection):
(WebCore::Editor::confirmComposition):
LayoutTests:
Add a regression test to ensure the same bug doesn't exist in textarea element.
* platform/mac/editing/input/selection-change-closes-typing-2-expected.txt: Copied from
LayoutTests/platform/mac/editing/input/selection-change-closes-typing-expected.txt.
* platform/mac/editing/input/selection-change-closes-typing-2.html: Copied from
LayoutTests/platform/mac/editing/input/selection-change-closes-typing.html.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (94496 => 94497)
--- trunk/LayoutTests/ChangeLog 2011-09-03 22:43:00 UTC (rev 94496)
+++ trunk/LayoutTests/ChangeLog 2011-09-03 23:30:08 UTC (rev 94497)
@@ -1,3 +1,17 @@
+2011-09-03 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r94274): selection-change-closes-typing.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=67377
+
+ Reviewed by Kent Tamura.
+
+ Add a regression test to ensure the same bug doesn't exist in textarea element.
+
+ * platform/mac/editing/input/selection-change-closes-typing-2-expected.txt: Copied from
+ LayoutTests/platform/mac/editing/input/selection-change-closes-typing-expected.txt.
+ * platform/mac/editing/input/selection-change-closes-typing-2.html: Copied from
+ LayoutTests/platform/mac/editing/input/selection-change-closes-typing.html.
+
2011-09-03 Mark Pilgrim <[email protected]>
Test that document.all.tags() matches IE behavior with too few arguments
Copied: trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2-expected.txt (from rev 94496, trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-expected.txt) (0 => 94497)
--- trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2-expected.txt (rev 0)
+++ trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2-expected.txt 2011-09-03 23:30:08 UTC (rev 94497)
@@ -0,0 +1,5 @@
+Test for bug 32905: With Pinyin Simplified IM, a wrong character is deleted from google.com suggestion.
+
+Should say PASS: PASS
+
+
Copied: trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2.html (from rev 94496, trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing.html) (0 => 94497)
--- trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2.html (rev 0)
+++ trunk/LayoutTests/platform/mac/editing/input/selection-change-closes-typing-2.html 2011-09-03 23:30:08 UTC (rev 94497)
@@ -0,0 +1,31 @@
+<html>
+<body>
+<p>Test for <a href="" 32905</a>:
+With Pinyin Simplified IM, a wrong character is deleted from google.com suggestion.</p>
+<p>Should say PASS: </p>
+<textarea id="test"></textarea>
+<script type="text/_javascript_">
+
+ if (window.layoutTestController)
+ layoutTestController.dumpAsText();
+
+ var testInput = document.getElementById("test");
+ testInput.focus();
+
+ if (window.layoutTestController) {
+
+ try {
+ textInputController.setMarkedText("P", 1, 0);
+ testInput.value="PAS";
+ eventSender.keyDown("S");
+
+ document.getElementsByTagName("p")[1].innerHTML += testInput.value;
+
+ } catch (ex) {
+ document.write("Exception: " + ex.description);
+ }
+ } else
+ document.write("This test only runs in automated mode<br>");
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (94496 => 94497)
--- trunk/Source/WebCore/ChangeLog 2011-09-03 22:43:00 UTC (rev 94496)
+++ trunk/Source/WebCore/ChangeLog 2011-09-03 23:30:08 UTC (rev 94497)
@@ -1,3 +1,23 @@
+2011-09-03 Ryosuke Niwa <[email protected]>
+
+ REGRESSION(r94274): selection-change-closes-typing.html fails
+ https://bugs.webkit.org/show_bug.cgi?id=67377
+
+ Reviewed by Kent Tamura.
+
+ The problem was that when the shadow DOM is updated by setInnerTextValue, WebKit layer detects the selection
+ change and calls confirmCompositionWithoutDisturbingSelection, which in turn modifies the shadow DOM by
+ inserting text.
+
+ Fixed the bug by not inserting text in confirmCompositionWithoutDisturbingSelection. It turned out that this
+ function is only used to cancel composition but never to confirming composition and restoring selection.
+
+ Test: platform/mac/editing/input/selection-change-closes-typing-2.html
+
+ * editing/Editor.cpp:
+ (WebCore::Editor::confirmCompositionWithoutDisturbingSelection):
+ (WebCore::Editor::confirmComposition):
+
2011-09-03 Sam Weinig <[email protected]>
Add missing Event constructors to DOMWindow.idl
Modified: trunk/Source/WebCore/editing/Editor.cpp (94496 => 94497)
--- trunk/Source/WebCore/editing/Editor.cpp 2011-09-03 22:43:00 UTC (rev 94496)
+++ trunk/Source/WebCore/editing/Editor.cpp 2011-09-03 23:30:08 UTC (rev 94497)
@@ -1472,11 +1472,13 @@
confirmComposition(m_compositionNode->data().substring(m_compositionStart, m_compositionEnd - m_compositionStart), false);
}
+// FIXME: This function is poorly named. Callers of this function uses this function to cancel composition,
+// and don't expect it to insert the current composition as the name suggests.
void Editor::confirmCompositionWithoutDisturbingSelection()
{
if (!m_compositionNode)
return;
- confirmComposition(m_compositionNode->data().substring(m_compositionStart, m_compositionEnd - m_compositionStart), true);
+ confirmComposition(emptyString(), true);
}
void Editor::confirmComposition(const String& text)
@@ -1490,10 +1492,11 @@
setIgnoreCompositionSelectionChange(true);
- VisibleSelection oldSelection = m_frame->selection()->selection();
+ if (preserveSelection)
+ ASSERT(text == emptyString());
+ else
+ selectComposition();
- selectComposition();
-
if (m_frame->selection()->isNone()) {
setIgnoreCompositionSelectionChange(false);
return;
@@ -1520,7 +1523,6 @@
insertTextForConfirmedComposition(text);
if (preserveSelection) {
- m_frame->selection()->setSelection(oldSelection, 0);
// An open typing command that disagrees about current selection would cause issues with typing later on.
TypingCommand::closeTyping(m_lastEditCommand.get());
}
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
