Changing selection shouldn't synchronously update editor UI components
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Feb 2014 00:12:59 +0000 (00:12 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Feb 2014 00:12:59 +0000 (00:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=129024

Reviewed by Brent Fulgham.

Source/WebCore:

Make updates to spellchecker, alternative text controller (correction pane), and delete button controller
asynchronous for programmatically triggered selection changes.

We continue to update their states synchronously immediately after we have applied, unapplied, or reapplied
editing commands to keep states in spell checker and alternative text controller consistent. We should be
able to make them asynchronous as well in the future but that should be done in a separate patch.

* WebCore.exp.in:
* editing/AlternativeTextController.cpp:
(WebCore::AlternativeTextController::respondToChangedSelection): This function used to enumerate all document
makers and call respondToMarkerAtEndOfWord on each one of them only to exit early when SetSelectionOptions
had DictationTriggered. This condition is now checked in Editor::respondToChangedSelection to avoid all the
unnecessary work and remove the dependency on SetSelectionOptions.
(WebCore::AlternativeTextController::respondToMarkerAtEndOfWord): Ditto.
* editing/AlternativeTextController.h:

* editing/Editor.cpp:
(WebCore::Editor::appliedEditing): Calls updateEditorUINowIfScheduled before calling respondToAppliedEditing
on the alternative text controller.
(WebCore::Editor::unappliedEditing): Ditto.
(WebCore::Editor::reappliedEditing): Ditto.
(WebCore::Editor::Editor): Initializes newly added booleans.
(WebCore::Editor::respondToChangedSelection): Continue to call respondToChangedSelection (for API consistency)
and setStartNewKillRingSequence but defer the "editor UI updates" to spellchecker, alternative text controller
and delete button controller by firing a newly added one shot timer.
(WebCore::Editor::updateEditorUINowIfScheduled): Synchronously update the pending editor UI updates.
(WebCore::Editor::editorUIUpdateTimerFired): Extracted from respondToChangedSelection.
* editing/Editor.h:

* testing/Internals.cpp:
(WebCore::Internals::markerCountForNode): Calls updateEditorUINowIfScheduled() to update document markers.
(WebCore::Internals::markerAt): Ditto.
(WebCore::Internals::updateEditorUINowIfScheduled): Added.
(WebCore::Internals::findEditingDeleteButton): Added. Updates delete button controller synchronously.
(WebCore::Internals::hasSpellingMarker): Calls updateEditorUINowIfScheduled() to update document markers.
(WebCore::Internals::hasAutocorrectedMarker): Ditto.
* testing/Internals.h:
* testing/Internals.idl:

Source/WebKit:

Added symbols for internals.

* WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in:

LayoutTests:

Many tests now calls internals.updateEditorUINowIfScheduled() to update the spellchecker states, and uses
setTimeout() to make things testable in the browser.

* editing/spelling/script-tests/spelling-backspace-between-lines.js:
(testTwoLinesMisspellings): Uses updateEditorUINowIfScheduled and setTimeout to make spellchecker recognize
two selection changes. This is okay since the user never moves selection multiple times in a single task.
* editing/spelling/spellcheck-attribute.html: Ditto.

* platform/mac/editing/deleting/deletionUI-click-on-delete-button.html: Use intenals.findEditingDeleteButton
which updates delete button controller states synchronously instead of getElementById which doesn't do that.
* platform/mac/editing/deleting/id-in-deletebutton-expected.txt:
* platform/mac/editing/deleting/id-in-deletebutton.html: Ditto. Also did some cleanups.
* platform/mac/editing/deleting/resources/deletionUI-helpers.js:
(deletionUIDeleteButtonForElement): Ditto.

* platform/mac/editing/spelling/editing-word-with-marker-1.html: Again, we must notify the spellchecker
synchronously here because we're expecting spellchecker to use the old selection set by setSelectionRange
in Editor::editorUIUpdateTimerFired triggered by the pasting command. This is, again, not a problem in
practice since user never pastes content synchronously after changing selection like this in a single task.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@164401 268f45cc-cd09-0410-ab3c-d52691b4dbfc

19 files changed:
LayoutTests/ChangeLog
LayoutTests/editing/spelling/script-tests/spelling-backspace-between-lines.js
LayoutTests/editing/spelling/spellcheck-attribute.html
LayoutTests/platform/mac/editing/deleting/deletionUI-click-on-delete-button.html
LayoutTests/platform/mac/editing/deleting/id-in-deletebutton-expected.txt
LayoutTests/platform/mac/editing/deleting/id-in-deletebutton.html
LayoutTests/platform/mac/editing/deleting/resources/deletionUI-helpers.js
LayoutTests/platform/mac/editing/spelling/editing-word-with-marker-1.html
Source/WebCore/ChangeLog
Source/WebCore/WebCore.exp.in
Source/WebCore/editing/AlternativeTextController.cpp
Source/WebCore/editing/AlternativeTextController.h
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebCore/testing/Internals.cpp
Source/WebCore/testing/Internals.h
Source/WebCore/testing/Internals.idl
Source/WebKit/ChangeLog
Source/WebKit/WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in

index 8873bf680ac9d9a81dd94e1d6c587cbe0df6f9d9..6e3bb6cbb27fc3367d149d79aff7c85e32986ff2 100644 (file)
@@ -1,3 +1,30 @@
+2014-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Changing selection shouldn't synchronously update editor UI components
+        https://bugs.webkit.org/show_bug.cgi?id=129024
+
+        Reviewed by Brent Fulgham.
+
+        Many tests now calls internals.updateEditorUINowIfScheduled() to update the spellchecker states, and uses
+        setTimeout() to make things testable in the browser.
+
+        * editing/spelling/script-tests/spelling-backspace-between-lines.js:
+        (testTwoLinesMisspellings): Uses updateEditorUINowIfScheduled and setTimeout to make spellchecker recognize
+        two selection changes. This is okay since the user never moves selection multiple times in a single task.
+        * editing/spelling/spellcheck-attribute.html: Ditto.
+
+        * platform/mac/editing/deleting/deletionUI-click-on-delete-button.html: Use intenals.findEditingDeleteButton
+        which updates delete button controller states synchronously instead of getElementById which doesn't do that.
+        * platform/mac/editing/deleting/id-in-deletebutton-expected.txt:
+        * platform/mac/editing/deleting/id-in-deletebutton.html: Ditto. Also did some cleanups.
+        * platform/mac/editing/deleting/resources/deletionUI-helpers.js:
+        (deletionUIDeleteButtonForElement): Ditto.
+
+        * platform/mac/editing/spelling/editing-word-with-marker-1.html: Again, we must notify the spellchecker
+        synchronously here because we're expecting spellchecker to use the old selection set by setSelectionRange
+        in Editor::editorUIUpdateTimerFired triggered by the pasting command. This is, again, not a problem in
+        practice since user never pastes content synchronously after changing selection like this in a single task.
+
 2014-02-19  Brent Fulgham  <bfulgham@apple.com>
 
         Another Windows update to quiet the bots.
index 0b1725aeff83ce23977313d96e843b4e10569302..5e4f47e3464dbb4cbddb6215250bbbf9766a1dd2 100644 (file)
@@ -33,15 +33,19 @@ function testTwoLinesMisspellings()
     window.sel = setup("target1"); // ^OK
 
     sel.modify("move", "forward", "line"); // ^OK zz OK
-    for (var i = 0; i < 3; i++)
-        sel.modify("move", "forward", "word");
-
-    shouldBeEqualToString("firstLineText('target1')", "OK");
-    shouldBeEqualToString("sel.anchorNode.data", "OK zz OK");
     if (window.internals)
-        shouldBecomeEqual("internals.hasSpellingMarker(3, 2)", "true", done);
-    else
-        done();
+        internals.updateEditorUINowIfScheduled();
+    setTimeout(function () {
+        for (var i = 0; i < 3; i++)
+            sel.modify("move", "forward", "word");
+
+        shouldBeEqualToString("firstLineText('target1')", "OK");
+        shouldBeEqualToString("sel.anchorNode.data", "OK zz OK");
+        if (window.internals)
+            shouldBecomeEqual("internals.hasSpellingMarker(3, 2)", "true", done);
+        else
+            done();
+    }, 100);
 }
 
 function testMisspellingsAfterLineMergeUsingDelete()
index f5b652942f5bb0738cf8a09314434037ce9d7e7e..99a0b9e12764726716ecf841be4e3d28e0fd11e1 100644 (file)
@@ -48,14 +48,18 @@ function testMarkerForMisspelledWord(parentId, shouldBeMisspelled, id, type, spe
     var input = addInputElement(parentId, id, type, spellcheck);
     input.focus();
     // Activate spellchecking.
-    moveSelectionForwardByWordCommand();
+    if (window.internals)
+        internals.updateEditorUINowIfScheduled();
+    setTimeout(function () {
+        moveSelectionForwardByWordCommand();
 
-    logMarkup(id, spellcheck, true);
+        logMarkup(id, spellcheck, true);
 
-    if (window.internals)
-        shouldBecomeEqual('internals.hasSpellingMarker(0, 2)', shouldBeMisspelled ? 'true' : 'false', done);
-    else
-        done();
+        if (window.internals)
+            shouldBecomeEqual('internals.hasSpellingMarker(0, 2)', shouldBeMisspelled ? 'true' : 'false', done);
+        else
+            done();
+    }, 10);
 }
 
 function logMarkup(id, spellcheckValue, shouldCloseTag)
index 556b420f63e9fad127ef3cfc2cd00bc1e277d842..1b41f513d0213911772d0e29176ee8760b50b6e3 100644 (file)
@@ -14,13 +14,13 @@ li = document.getElementById("li");
 sel.setPosition(li, 0);
 
 if (window.testRunner) {
-    deleteButton = document.getElementById("WebKit-Editing-Delete-Button");
+    deleteButton = internals.findEditingDeleteButton();
     x = deleteButton.offsetParent.offsetLeft + deleteButton.offsetParent.offsetParent.offsetLeft + deleteButton.offsetLeft + deleteButton.offsetWidth / 2;
     y = deleteButton.offsetParent.offsetTop + deleteButton.offsetParent.offsetParent.offsetTop + deleteButton.offsetTop + deleteButton.offsetHeight / 2;
     eventSender.mouseMoveTo(x, y);
     eventSender.mouseDown();
     eventSender.mouseUp();
-    deleteButton = document.getElementById("WebKit-Editing-Delete-Button");
+    deleteButton = internals.findEditingDeleteButton();
     testContainer = document.getElementById("test");
     Markup.description("There should be no visible content in the markup below. This test is for a bug where the delete button wouldn't work because it had -webkit-user-select:none instead of -webkit-user-select:ignore.");
     if (deleteButton)
index d1496fe065eeabeec203d2d8e5b2583780c9c943..914f803d2ed1dede8b64a977c680c8f00ccc325b 100644 (file)
@@ -3,7 +3,7 @@ Test document.getElementById("WebKit-Editing-Delete-Button")
 On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
 
 
-PASS deleteButton is non-null.
+PASS internals.findEditingDeleteButton(); document.getElementById("WebKit-Editing-Delete-Button") is non-null.
 PASS successfullyParsed is true
 
 TEST COMPLETE
index 0d6383b3eaf1282571dc26938033be1eab4a42d4..55036c1e229f45a73a20e0d92acfb99db7795db6 100644 (file)
@@ -1,12 +1,8 @@
 <html>
-<head>
-<script src="../../../../resources/js-test-pre.js"></script>
-</head>
 <body>
-
 <div contenteditable="true">
 <ul class="needsDeletionUI"><li>1</li><li id="li">2</li></ul>
-
+<script src="../../../../resources/js-test-pre.js"></script>
 <script>
 description('Test document.getElementById("WebKit-Editing-Delete-Button")');
 
@@ -14,10 +10,11 @@ sel = window.getSelection();
 li = document.getElementById("li");
 sel.setPosition(li, 0);
 
-if (window.testRunner) {
-    deleteButton = document.getElementById("WebKit-Editing-Delete-Button");
-    shouldBeNonNull('deleteButton');
-}
+if (window.testRunner)
+    shouldBeNonNull('internals.findEditingDeleteButton(); document.getElementById("WebKit-Editing-Delete-Button")');
+
+var successfullyParsed = true;
+
 </script>
 <script src="../../../../resources/js-test-post.js"></script>
 </body>
index 487246ab072065e71c89fbe312778e2e102c4392..310988ee5cf430bf071be53f4492d20f30ab5fbb 100644 (file)
@@ -10,11 +10,12 @@ function debug(msg)
 
 function deletionUIDeleteButtonForElement(id)
 {
+    if (!window.internals)
+        return null;
     var sel = window.getSelection();
     var selElement = document.getElementById(id);
     sel.setPosition(selElement, 0);
-    var deleteButton = document.getElementById("WebKit-Editing-Delete-Button");
-    return deleteButton;
+    return internals.findEditingDeleteButton();
 }
 
 function determineDeletionUIExistence(id)
index fcedd2a36233cc95cf9c24faae1858b35467a25e..2539c17c6ec51dc89624f5069cd0f6e57b4866bd 100644 (file)
@@ -148,6 +148,8 @@ textarea = document.getElementById('test');
 textarea.setSelectionRange(0,4);
 execCopyCommand();
 textarea.setSelectionRange(15, 15);
+if (window.internals)
+    internals.updateEditorUINowIfScheduled();
 execPasteCommand();
 if (window.internals && window.internals.hasSpellingMarker) {
     if (window.internals.hasSpellingMarker(7,8) == 0) {
index e7b04647c1dae0a5f0398f92c3f9c3d89f85d89d..ffb5c2a251fb3c41b21a84ccbd00f3e562cd8a04 100644 (file)
@@ -1,3 +1,49 @@
+2014-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Changing selection shouldn't synchronously update editor UI components
+        https://bugs.webkit.org/show_bug.cgi?id=129024
+
+        Reviewed by Brent Fulgham.
+
+        Make updates to spellchecker, alternative text controller (correction pane), and delete button controller
+        asynchronous for programmatically triggered selection changes.
+
+        We continue to update their states synchronously immediately after we have applied, unapplied, or reapplied
+        editing commands to keep states in spell checker and alternative text controller consistent. We should be
+        able to make them asynchronous as well in the future but that should be done in a separate patch.
+
+        * WebCore.exp.in:
+        * editing/AlternativeTextController.cpp:
+        (WebCore::AlternativeTextController::respondToChangedSelection): This function used to enumerate all document
+        makers and call respondToMarkerAtEndOfWord on each one of them only to exit early when SetSelectionOptions
+        had DictationTriggered. This condition is now checked in Editor::respondToChangedSelection to avoid all the
+        unnecessary work and remove the dependency on SetSelectionOptions.
+        (WebCore::AlternativeTextController::respondToMarkerAtEndOfWord): Ditto.
+        * editing/AlternativeTextController.h:
+
+        * editing/Editor.cpp:
+        (WebCore::Editor::appliedEditing): Calls updateEditorUINowIfScheduled before calling respondToAppliedEditing
+        on the alternative text controller.
+        (WebCore::Editor::unappliedEditing): Ditto.
+        (WebCore::Editor::reappliedEditing): Ditto.
+        (WebCore::Editor::Editor): Initializes newly added booleans.
+        (WebCore::Editor::respondToChangedSelection): Continue to call respondToChangedSelection (for API consistency)
+        and setStartNewKillRingSequence but defer the "editor UI updates" to spellchecker, alternative text controller
+        and delete button controller by firing a newly added one shot timer.
+        (WebCore::Editor::updateEditorUINowIfScheduled): Synchronously update the pending editor UI updates.
+        (WebCore::Editor::editorUIUpdateTimerFired): Extracted from respondToChangedSelection.
+        * editing/Editor.h:
+
+        * testing/Internals.cpp:
+        (WebCore::Internals::markerCountForNode): Calls updateEditorUINowIfScheduled() to update document markers.
+        (WebCore::Internals::markerAt): Ditto.
+        (WebCore::Internals::updateEditorUINowIfScheduled): Added.
+        (WebCore::Internals::findEditingDeleteButton): Added. Updates delete button controller synchronously.
+        (WebCore::Internals::hasSpellingMarker): Calls updateEditorUINowIfScheduled() to update document markers.
+        (WebCore::Internals::hasAutocorrectedMarker): Ditto.
+        * testing/Internals.h:
+        * testing/Internals.idl:
+
 2014-02-19  Anders Carlsson  <andersca@apple.com>
 
         Add WTF_MAKE_FAST_ALLOCATED to more classes
index 0fb67e9defca7b5403c125c91c11aeab412df162..5ba8637e5710877cca546d2153878d28a11a81d0 100644 (file)
@@ -1116,6 +1116,7 @@ __ZN7WebCore6Editor26increaseSelectionListLevelEv
 __ZN7WebCore6Editor26toggleOverwriteModeEnabledEv
 __ZN7WebCore6Editor26writeSelectionToPasteboardERNS_10PasteboardE
 __ZN7WebCore6Editor28replaceSelectionWithFragmentEN3WTF10PassRefPtrINS_16DocumentFragmentEEEbbb
+__ZN7WebCore6Editor28updateEditorUINowIfScheduledEv
 __ZN7WebCore6Editor29canDecreaseSelectionListLevelEv
 __ZN7WebCore6Editor29canIncreaseSelectionListLevelEv
 __ZN7WebCore6Editor29handleAlternativeTextUIResultERKN3WTF6StringE
index 4ad3dc8ae4d9a6a008a2bf375ce1aa47f70cb727..2e7ec462aeea61b8b8523a7b3ef9d9701ac48c75 100644 (file)
@@ -440,7 +440,7 @@ FloatRect AlternativeTextController::rootViewRectForRange(const Range* range) co
     return view->contentsToRootView(IntRect(boundingRect));
 }        
 
-void AlternativeTextController::respondToChangedSelection(const VisibleSelection& oldSelection, FrameSelection::SetSelectionOptions options)
+void AlternativeTextController::respondToChangedSelection(const VisibleSelection& oldSelection)
 {
     VisibleSelection currentSelection(m_frame.selection().selection());
     // When user moves caret to the end of autocorrected word and pauses, we show the panel
@@ -473,7 +473,7 @@ void AlternativeTextController::respondToChangedSelection(const VisibleSelection
         if (!marker)
             continue;
 
-        if (respondToMarkerAtEndOfWord(*marker, position, options))
+        if (respondToMarkerAtEndOfWord(*marker, position))
             break;
     }
 }
@@ -625,10 +625,8 @@ bool AlternativeTextController::shouldStartTimerFor(const WebCore::DocumentMarke
     return (((marker.type() == DocumentMarker::Replacement && !marker.description().isNull()) || marker.type() == DocumentMarker::Spelling || marker.type() == DocumentMarker::DictationAlternatives) && static_cast<int>(marker.endOffset()) == endOffset);
 }
 
-bool AlternativeTextController::respondToMarkerAtEndOfWord(const DocumentMarker& marker, const Position& endOfWordPosition, FrameSelection::SetSelectionOptions options)
+bool AlternativeTextController::respondToMarkerAtEndOfWord(const DocumentMarker& marker, const Position& endOfWordPosition)
 {
-    if (options & FrameSelection::DictationTriggered)
-        return false;
     if (!shouldStartTimerFor(marker, endOfWordPosition.offsetInContainerNode()))
         return false;
     Node* node = endOfWordPosition.containerNode();
index c843f603df753169644dcde2f8e5a63cddd3b833..e570564da17adcc63f8b6971e5b6db5cc41ada32 100644 (file)
@@ -108,7 +108,7 @@ public:
     void respondToUnappliedSpellCorrection(const VisibleSelection&, const String& corrected, const String& correction) UNLESS_ENABLED({ UNUSED_PARAM(corrected); UNUSED_PARAM(correction); })
     void respondToAppliedEditing(CompositeEditCommand*) UNLESS_ENABLED({ })
     void respondToUnappliedEditing(EditCommandComposition*) UNLESS_ENABLED({ })
-    void respondToChangedSelection(const VisibleSelection& oldSelection, FrameSelection::SetSelectionOptions) UNLESS_ENABLED({ UNUSED_PARAM(oldSelection); })
+    void respondToChangedSelection(const VisibleSelection& oldSelection) UNLESS_ENABLED({ UNUSED_PARAM(oldSelection); })
 
     void stopPendingCorrection(const VisibleSelection& oldSelection) UNLESS_ENABLED({ UNUSED_PARAM(oldSelection); })
     void applyPendingCorrection(const VisibleSelection& selectionAfterTyping) UNLESS_ENABLED({ UNUSED_PARAM(selectionAfterTyping); })
@@ -144,7 +144,7 @@ private:
     String markerDescriptionForAppliedAlternativeText(AlternativeTextType, DocumentMarker::MarkerType);
 
     bool shouldStartTimerFor(const DocumentMarker&, int endOffset) const;
-    bool respondToMarkerAtEndOfWord(const DocumentMarker&, const Position& endOfWordPosition, FrameSelection::SetSelectionOptions);
+    bool respondToMarkerAtEndOfWord(const DocumentMarker&, const Position& endOfWordPosition);
 
     AlternativeTextClient* alternativeTextClient();
     EditorClient* editorClient();
index f93215e18410f350db98b11b39b6136a43bfe8a6..8982596892209d97bcc2a3de52a2b09a2e35f425 100644 (file)
@@ -1066,8 +1066,6 @@ void Editor::appliedEditing(PassRefPtr<CompositeEditCommand> cmd)
     ASSERT(composition);
     VisibleSelection newSelection(cmd->endingSelection());
 
-    m_alternativeTextController->respondToAppliedEditing(cmd.get());
-
     notifyTextFromControls(composition->startingRootEditableElement(), composition->endingRootEditableElement());
 
     // Don't clear the typing style with this selection change.  We do those things elsewhere if necessary.
@@ -1075,6 +1073,10 @@ void Editor::appliedEditing(PassRefPtr<CompositeEditCommand> cmd)
     changeSelectionAfterCommand(newSelection, options);
     dispatchEditableContentChangedEvents(composition->startingRootEditableElement(), composition->endingRootEditableElement());
 
+    updateEditorUINowIfScheduled();
+    
+    m_alternativeTextController->respondToAppliedEditing(cmd.get());
+
     if (!cmd->preservesTypingStyle())
         m_frame.selection().clearTypingStyle();
 
@@ -1102,6 +1104,8 @@ void Editor::unappliedEditing(PassRefPtr<EditCommandComposition> cmd)
     changeSelectionAfterCommand(newSelection, FrameSelection::defaultSetSelectionOptions());
     dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(), cmd->endingRootEditableElement());
 
+    updateEditorUINowIfScheduled();
+
     m_alternativeTextController->respondToUnappliedEditing(cmd.get());
 
     m_lastEditCommand = 0;
@@ -1119,6 +1123,8 @@ void Editor::reappliedEditing(PassRefPtr<EditCommandComposition> cmd)
     VisibleSelection newSelection(cmd->endingSelection());
     changeSelectionAfterCommand(newSelection, FrameSelection::defaultSetSelectionOptions());
     dispatchEditableContentChangedEvents(cmd->startingRootEditableElement(), cmd->endingRootEditableElement());
+    
+    updateEditorUINowIfScheduled();
 
     m_lastEditCommand = 0;
     if (client())
@@ -1141,6 +1147,9 @@ Editor::Editor(Frame& frame)
     , m_areMarkedTextMatchesHighlighted(false)
     , m_defaultParagraphSeparator(EditorParagraphSeparatorIsDiv)
     , m_overwriteModeEnabled(false)
+    , m_editorUIUpdateTimer(this, &Editor::editorUIUpdateTimerFired)
+    , m_editorUIUpdateTimerShouldCheckSpellingAndGrammar(false)
+    , m_editorUIUpdateTimerWasTriggeredByDictation(false)
 {
 }
 
@@ -3294,7 +3303,7 @@ void Editor::setMarkedTextMatchesAreHighlighted(bool flag)
     document().markers().repaintMarkers(DocumentMarker::TextMatch);
 }
 
-void Editor::respondToChangedSelection(const VisibleSelection& oldSelection, FrameSelection::SetSelectionOptions options)
+void Editor::respondToChangedSelection(const VisibleSelection&, FrameSelection::SetSelectionOptions options)
 {
 #if PLATFORM(IOS)
     // FIXME: Should suppress selection change notifications during a composition change <https://webkit.org/b/38830> 
@@ -3302,9 +3311,34 @@ void Editor::respondToChangedSelection(const VisibleSelection& oldSelection, Fra
         return;
 #endif
 
-    m_alternativeTextController->stopPendingCorrection(oldSelection);
+    if (client())
+        client()->respondToChangedSelection(&m_frame);
+    setStartNewKillRingSequence(true);
+
+    if (m_editorUIUpdateTimer.isActive())
+        return;
+
+    // Don't check spelling and grammar if the change of selection is triggered by spelling correction itself.
+    m_editorUIUpdateTimerShouldCheckSpellingAndGrammar = options & FrameSelection::CloseTyping
+        && !(options & FrameSelection::SpellCorrectionTriggered);
+    m_editorUIUpdateTimerWasTriggeredByDictation = options & FrameSelection::DictationTriggered;
+    m_editorUIUpdateTimer.startOneShot(0);
+}
+
+void Editor::updateEditorUINowIfScheduled()
+{
+    if (!m_editorUIUpdateTimer.isActive())
+        return;
+    m_editorUIUpdateTimer.stop();
+    editorUIUpdateTimerFired(m_editorUIUpdateTimer);
+}
+
+void Editor::editorUIUpdateTimerFired(Timer<Editor>&)
+{
+    VisibleSelection oldSelection = m_oldSelectionForEditorUIUpdate;
 
-    bool closeTyping = options & FrameSelection::CloseTyping;
+    m_alternativeTextController->stopPendingCorrection(oldSelection);
+    
     bool isContinuousSpellCheckingEnabled = this->isContinuousSpellCheckingEnabled();
     bool isContinuousGrammarCheckingEnabled = isContinuousSpellCheckingEnabled && isGrammarCheckingEnabled();
     if (isContinuousSpellCheckingEnabled) {
@@ -3331,13 +3365,10 @@ void Editor::respondToChangedSelection(const VisibleSelection& oldSelection, Fra
                 newSelectedSentence = VisibleSelection(startOfSentence(newStart), endOfSentence(newStart));
         }
 
-        // Don't check spelling and grammar if the change of selection is triggered by spelling correction itself.
-        bool shouldCheckSpellingAndGrammar = !(options & FrameSelection::SpellCorrectionTriggered);
-
         // When typing we check spelling elsewhere, so don't redo it here.
         // If this is a change in selection resulting from a delete operation,
         // oldSelection may no longer be in the document.
-        if (shouldCheckSpellingAndGrammar && closeTyping && oldSelection.isContentEditable() && oldSelection.start().deprecatedNode() && oldSelection.start().anchorNode()->inDocument()) {
+        if (m_editorUIUpdateTimerShouldCheckSpellingAndGrammar && oldSelection.isContentEditable() && oldSelection.start().deprecatedNode() && oldSelection.start().anchorNode()->inDocument()) {
             VisiblePosition oldStart(oldSelection.visibleStart());
             VisibleSelection oldAdjacentWords = VisibleSelection(startOfWord(oldStart, LeftWordIfOnBoundary), endOfWord(oldStart, RightWordIfOnBoundary));
             if (oldAdjacentWords != newAdjacentWords) {
@@ -3365,13 +3396,13 @@ void Editor::respondToChangedSelection(const VisibleSelection& oldSelection, Fra
     if (!isContinuousGrammarCheckingEnabled)
         document().markers().removeMarkers(DocumentMarker::Grammar);
 
-    if (client())
-        client()->respondToChangedSelection(&m_frame);
-    setStartNewKillRingSequence(true);
 #if ENABLE(DELETION_UI)
     m_deleteButtonController->respondToChangedSelection(oldSelection);
 #endif
-    m_alternativeTextController->respondToChangedSelection(oldSelection, options);
+    if (m_editorUIUpdateTimerWasTriggeredByDictation)
+        m_alternativeTextController->respondToChangedSelection(oldSelection);
+
+    m_oldSelectionForEditorUIUpdate = m_frame.selection().selection();
 }
 
 static Node* findFirstMarkable(Node* node)
index a74ffe89f3435bd656bc0c21f6c8dd00f9a46482..c5672604627ddac2e0664142f48fded9f2479e49 100644 (file)
@@ -362,6 +362,7 @@ public:
     IntRect firstRectForRange(Range*) const;
 
     void respondToChangedSelection(const VisibleSelection& oldSelection, FrameSelection::SetSelectionOptions);
+    void updateEditorUINowIfScheduled();
     bool shouldChangeSelection(const VisibleSelection& oldSelection, const VisibleSelection& newSelection, EAffinity, bool stillSelecting) const;
     unsigned countMatchesForText(const String&, Range*, FindOptions, unsigned limit, bool markMatches, Vector<RefPtr<Range>>*);
     bool markedTextMatchesAreHighlighted() const;
@@ -464,6 +465,8 @@ private:
 
     void changeSelectionAfterCommand(const VisibleSelection& newSelection, FrameSelection::SetSelectionOptions);
 
+    void editorUIUpdateTimerFired(Timer<Editor>&);
+
     Node* findEventTargetFromSelection() const;
 
     bool unifiedTextCheckerEnabled() const;
@@ -495,6 +498,11 @@ private:
     bool m_areMarkedTextMatchesHighlighted;
     EditorParagraphSeparator m_defaultParagraphSeparator;
     bool m_overwriteModeEnabled;
+
+    VisibleSelection m_oldSelectionForEditorUIUpdate;
+    Timer<Editor> m_editorUIUpdateTimer;
+    bool m_editorUIUpdateTimerShouldCheckSpellingAndGrammar;
+    bool m_editorUIUpdateTimerWasTriggeredByDictation;
 };
 
 inline void Editor::setStartNewKillRingSequence(bool flag)
index 66e620b8f3f796f978a2821c550f8133efe933ed..5e815ceaf36783bef034ae86687285c267d99149 100644 (file)
@@ -748,6 +748,8 @@ unsigned Internals::markerCountForNode(Node* node, const String& markerType, Exc
         return 0;
     }
 
+    node->document().frame()->editor().updateEditorUINowIfScheduled();
+
     return node->document().markers().markersFor(node, markerTypes).size();
 }
 
@@ -765,6 +767,8 @@ DocumentMarker* Internals::markerAt(Node* node, const String& markerType, unsign
         return 0;
     }
 
+    node->document().frame()->editor().updateEditorUINowIfScheduled();
+
     Vector<DocumentMarker*> markers = node->document().markers().markersFor(node, markerTypes);
     if (markers.size() <= index)
         return 0;
@@ -1295,12 +1299,34 @@ void Internals::setDeviceProximity(const String& eventType, double value, double
 #endif
 }
 
+void Internals::updateEditorUINowIfScheduled()
+{
+    if (Document* document = contextDocument()) {
+        if (Frame* frame = document->frame())
+            frame->editor().updateEditorUINowIfScheduled();
+    }
+}
+
+Node* Internals::findEditingDeleteButton()
+{
+    Document* document = contextDocument();
+    if (!document || !document->frame())
+        return 0;
+
+    updateEditorUINowIfScheduled();
+
+    // FIXME: We shouldn't pollute the id namespace with this name.
+    return document->getElementById("WebKit-Editing-Delete-Button");
+}
+
 bool Internals::hasSpellingMarker(int from, int length, ExceptionCode&)
 {
     Document* document = contextDocument();
     if (!document || !document->frame())
         return 0;
 
+    updateEditorUINowIfScheduled();
+
     return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Spelling, from, length);
 }
     
@@ -1309,7 +1335,9 @@ bool Internals::hasAutocorrectedMarker(int from, int length, ExceptionCode&)
     Document* document = contextDocument();
     if (!document || !document->frame())
         return 0;
-    
+
+    updateEditorUINowIfScheduled();
+
     return document->frame()->editor().selectionStartHasMarkerFor(DocumentMarker::Autocorrected, from, length);
 }
 
index 06b31b3ab5bb93d532813bb0672f1fb229be5da9..2c42da70d4385b266ab1aceff5a64a9c2faa6740 100644 (file)
@@ -169,6 +169,9 @@ public:
 
     String parserMetaData(Deprecated::ScriptValue = Deprecated::ScriptValue());
 
+    Node* findEditingDeleteButton();
+    void updateEditorUINowIfScheduled();
+
     bool hasSpellingMarker(int from, int length, ExceptionCode&);
     bool hasGrammarMarker(int from, int length, ExceptionCode&);
     bool hasAutocorrectedMarker(int from, int length, ExceptionCode&);
index 58d850fff5070a5a818ddb25e293e5cbd18ec4cf..505475f9be1af179b98dded28d8a3ad5a8bae682 100644 (file)
     // Calling parserMetaData() with no arguments gets the metadata for the script of the current scope.
     DOMString parserMetaData(optional any func);
 
+    void updateEditorUINowIfScheduled();
+
+    Node findEditingDeleteButton();
+
     [RaisesException] boolean hasSpellingMarker(long from, long length);
     [RaisesException] boolean hasGrammarMarker(long from, long length);
     [RaisesException] boolean hasAutocorrectedMarker(long from, long length);
index 06c849bc21eaa88e56e66928fb20e3b6acb1a6a8..31e9f675343b0181fa474929b561e26c9fa09bfd 100644 (file)
@@ -1,3 +1,14 @@
+2014-02-18  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Changing selection shouldn't synchronously update editor UI components
+        https://bugs.webkit.org/show_bug.cgi?id=129024
+
+        Reviewed by Brent Fulgham.
+
+        Added symbols for internals.
+
+        * WebKit.vcxproj/WebKitExportGenerator/WebKitExports.def.in:
+
 2014-02-17  Sergio Correia  <sergio.correia@openbossa.org>
 
         Replace uses of PassOwnPtr/OwnPtr with std::unique_ptr in WebCore/inspector
index 690c89fd678f45cc7c2a97b5382227531ad66ed9..49c26094cfccf94226a1e17ad75e92b0a6c262e8 100644 (file)
@@ -204,6 +204,7 @@ EXPORTS
         symbolWithPointer(?garbageCollectDocumentResources@CachedResourceLoader@WebCore@@QAEXXZ, ?garbageCollectDocumentResources@CachedResourceLoader@WebCore@@QEAAXXZ)
         symbolWithPointer(?getCachedDOMStructure@WebCore@@YAPAVStructure@JSC@@PAVJSDOMGlobalObject@1@PBUClassInfo@3@@Z, ?getCachedDOMStructure@WebCore@@YAPEAVStructure@JSC@@PEAVJSDOMGlobalObject@1@PEBUClassInfo@3@@Z)
         symbolWithPointer(?getData16SlowCase@StringImpl@WTF@@ABEPB_WXZ, ?getData16SlowCase@StringImpl@WTF@@AEBAPEB_WXZ)
+        symbolWithPointer(?getElementById@TreeScope@WebCore@@QBEPAVElement@2@ABVAtomicString@WTF@@@Z)
         symbolWithPointer(?getLocationAndLengthFromRange@TextIterator@WebCore@@SA_NPAVNode@2@PBVRange@2@AAI2@Z, ?getLocationAndLengthFromRange@TextIterator@WebCore@@SA_NPEAVNode@2@PEBVRange@2@AEA_K2@Z)
         symbolWithPointer(?hitTest@RenderView@WebCore@@QAE_NABVHitTestRequest@2@AAVHitTestResult@2@@Z, ?hitTest@RenderView@WebCore@@QEAA_NAEBVHitTestRequest@2@AEAVHitTestResult@2@@Z)
         ?inputTag@HTMLNames@WebCore@@3VQualifiedName@2@B
@@ -323,6 +324,7 @@ EXPORTS
         symbolWithPointer(?toJS@WebCore@@YA?AVJSValue@JSC@@PAVExecState@3@PAVJSDOMGlobalObject@1@PAVNodeList@1@@Z, ?toJS@WebCore@@YA?AVJSValue@JSC@@PEAVExecState@3@PEAVJSDOMGlobalObject@1@PEAVNodeList@1@@Z)
         symbolWithPointer(?toRange@WebCore@@YAPAVRange@1@VJSValue@JSC@@@Z, ?toRange@WebCore@@YAPEAVRange@1@VJSValue@JSC@@@Z)
         symbolWithPointer(?isTreeScope@Node@WebCore@@QBE_NXZ, ?isTreeScope@Node@WebCore@@QEBA_NXZ)
+        symbolWithPointer(?updateEditorUINowIfScheduled@Editor@WebCore@@QAEXXZ)
         symbolWithPointer(?updateLayoutIgnorePendingStylesheets@Document@WebCore@@QAEXXZ, ?updateLayoutIgnorePendingStylesheets@Document@WebCore@@QEAAXXZ)
         symbolWithPointer(?updateStyleIfNeeded@Document@WebCore@@QAEXXZ, ?updateStyleIfNeeded@Document@WebCore@@QEAAXXZ)
         symbolWithPointer(?view@Document@WebCore@@QBEPAVFrameView@2@XZ, ?view@Document@WebCore@@QEBAPEAVFrameView@2@XZ)