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 8873bf6..6e3bb6c 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 0b1725a..5e4f47e 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 f5b6529..99a0b9e 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 556b420..1b41f51 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 d1496fe..914f803 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 0d6383b..55036c1 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 487246a..310988e 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 fcedd2a..2539c17 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 e7b0464..ffb5c2a 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 0fb67e9..5ba8637 100644 (file)
@@ -1116,6 +1116,7 @@ __ZN7WebCore6Editor26increaseSelectionListLevelEv
 __ZN7WebCore6Editor26toggleOverwriteModeEnabledEv
 __ZN7WebCore6Editor26writeSelectionToPasteboardERNS_10PasteboardE
 __ZN7WebCore6Editor28replaceSelectionWithFragmentEN3WTF10PassRefPtrINS_16DocumentFragmentEEEbbb
+__ZN7WebCore6Editor28updateEditorUINowIfScheduledEv
 __ZN7WebCore6Editor29canDecreaseSelectionListLevelEv
 __ZN7WebCore6Editor29canIncreaseSelectionListLevelEv
 __ZN7WebCore6Editor29handleAlternativeTextUIResultERKN3WTF6StringE
index 4ad3dc8..2e7ec46 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 c843f60..e570564 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 f93215e..8982596 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 a74ffe8..c567260 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 66e620b..5e815ce 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 06b31b3..2c42da7 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 58d850f..505475f 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 06c849b..31e9f67 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 690c89f..49c2609 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)