[WK1] Candidate visibility should not update as a result of selection during a dictio...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Oct 2016 23:19:19 +0000 (23:19 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 31 Oct 2016 23:19:19 +0000 (23:19 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164236
<rdar://problem/28747712>

Reviewed by Beth Dakin.

Source/WebCore:

Moves state that keeps track of whether or not a dictionary lookup is active from the WebPage to the Editor, so
that when clicking a text field or contenteditable in WK1 or WK2, we are able to avoid notifying the
WebEditorClient of the selection change.

Changes to WK2 are covered by existing unit tests in WKWebViewCandidateTests which verify that clicking does not
thrash candidate list visibility. A similar test will be added in the future for the WK1 case in CandidateTests.

* editing/Editor.h:
(WebCore::Editor::setIsGettingDictionaryPopupInfo):
(WebCore::Editor::isGettingDictionaryPopupInfo):

Source/WebKit/mac:

See WebCore ChangeLog for more detail. Sets the Editor's isGettingDictionaryPopupInfo state to true during a
dictionary lookup.

* WebCoreSupport/WebEditorClient.mm:
(WebEditorClient::respondToChangedSelection):
* WebView/WebImmediateActionController.mm:
(+[WebImmediateActionController _dictionaryPopupInfoForRange:inFrame:withLookupOptions:indicatorOptions:transition:]):

Source/WebKit2:

See WebCore ChangeLog for more detail. Removes m_isGettingDictionaryPopupInfo from the WebPage in favor of
keeping track of the same state in Editor, so that both the WK1 and WK2 cases can share the same codepath.

* WebProcess/WebPage/WebPage.cpp:
(WebKit::WebPage::didChangeSelection):
* WebProcess/WebPage/WebPage.h:
* WebProcess/WebPage/mac/WebPageMac.mm:
(WebKit::WebPage::dictionaryPopupInfoForRange):

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

Source/WebCore/ChangeLog
Source/WebCore/editing/Editor.h
Source/WebKit/mac/ChangeLog
Source/WebKit/mac/WebCoreSupport/WebEditorClient.mm
Source/WebKit/mac/WebView/WebImmediateActionController.mm
Source/WebKit2/ChangeLog
Source/WebKit2/WebProcess/WebPage/WebPage.cpp
Source/WebKit2/WebProcess/WebPage/WebPage.h
Source/WebKit2/WebProcess/WebPage/mac/WebPageMac.mm

index b4b3009..09ab55f 100644 (file)
@@ -1,3 +1,22 @@
+2016-10-31  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK1] Candidate visibility should not update as a result of selection during a dictionary lookup
+        https://bugs.webkit.org/show_bug.cgi?id=164236
+        <rdar://problem/28747712>
+
+        Reviewed by Beth Dakin.
+
+        Moves state that keeps track of whether or not a dictionary lookup is active from the WebPage to the Editor, so
+        that when clicking a text field or contenteditable in WK1 or WK2, we are able to avoid notifying the
+        WebEditorClient of the selection change.
+
+        Changes to WK2 are covered by existing unit tests in WKWebViewCandidateTests which verify that clicking does not
+        thrash candidate list visibility. A similar test will be added in the future for the WK1 case in CandidateTests.
+
+        * editing/Editor.h:
+        (WebCore::Editor::setIsGettingDictionaryPopupInfo):
+        (WebCore::Editor::isGettingDictionaryPopupInfo):
+
 2016-10-31  Antoine Quint  <graouts@apple.com>
 
         [Modern Media Controls] Media Controller: Placard support
index e234a8a..8670ae3 100644 (file)
@@ -476,6 +476,9 @@ public:
     RefPtr<Range> rangeForTextCheckingResult(const TextCheckingResult&) const;
     bool isHandlingAcceptedCandidate() const { return m_isHandlingAcceptedCandidate; }
 
+    void setIsGettingDictionaryPopupInfo(bool b) { m_isGettingDictionaryPopupInfo = b; }
+    bool isGettingDictionaryPopupInfo() const { return m_isGettingDictionaryPopupInfo; }
+
 private:
     class WebContentReader;
 
@@ -551,6 +554,8 @@ private:
     Timer m_telephoneNumberDetectionUpdateTimer;
     Vector<RefPtr<Range>> m_detectedTelephoneNumberRanges;
 #endif
+
+    bool m_isGettingDictionaryPopupInfo { false };
 };
 
 inline void Editor::setStartNewKillRingSequence(bool flag)
index c6e488e..cd7dd5a 100644 (file)
@@ -1,3 +1,19 @@
+2016-10-31  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK1] Candidate visibility should not update as a result of selection during a dictionary lookup
+        https://bugs.webkit.org/show_bug.cgi?id=164236
+        <rdar://problem/28747712>
+
+        Reviewed by Beth Dakin.
+
+        See WebCore ChangeLog for more detail. Sets the Editor's isGettingDictionaryPopupInfo state to true during a
+        dictionary lookup.
+
+        * WebCoreSupport/WebEditorClient.mm:
+        (WebEditorClient::respondToChangedSelection):
+        * WebView/WebImmediateActionController.mm:
+        (+[WebImmediateActionController _dictionaryPopupInfoForRange:inFrame:withLookupOptions:indicatorOptions:transition:]):
+
 2016-10-31  Darin Adler  <darin@apple.com>
 
         Move ChildNode and ParentNode from ExceptionCode to Exception, add support for ExceptionOr<T&>
index e78b399..da49da9 100644 (file)
@@ -341,6 +341,9 @@ void WebEditorClient::respondToChangedContents()
 
 void WebEditorClient::respondToChangedSelection(Frame* frame)
 {
+    if (frame->editor().isGettingDictionaryPopupInfo())
+        return;
+
     NSView<WebDocumentView> *documentView = [[kit(frame) frameView] documentView];
     if ([documentView isKindOfClass:[WebHTMLView class]]) {
         [(WebHTMLView *)documentView _selectionChanged];
index d6b2038..ad98356 100644 (file)
@@ -39,6 +39,7 @@
 #import <WebCore/DataDetection.h>
 #import <WebCore/DataDetectorsSPI.h>
 #import <WebCore/DictionaryLookup.h>
+#import <WebCore/Editor.h>
 #import <WebCore/EventHandler.h>
 #import <WebCore/FocusController.h>
 #import <WebCore/Frame.h>
@@ -501,19 +502,26 @@ static IntRect elementBoundingBoxInWindowCoordinatesFromNode(Node* node)
 
 + (DictionaryPopupInfo)_dictionaryPopupInfoForRange:(Range&)range inFrame:(Frame*)frame withLookupOptions:(NSDictionary *)lookupOptions indicatorOptions:(TextIndicatorOptions)indicatorOptions transition:(TextIndicatorPresentationTransition)presentationTransition
 {
+    Editor& editor = frame->editor();
+    editor.setIsGettingDictionaryPopupInfo(true);
+
     // Dictionary API will accept a whitespace-only string and display UI as if it were real text,
     // so bail out early to avoid that.
     DictionaryPopupInfo popupInfo;
-    if (range.text().stripWhiteSpace().isEmpty())
+    if (range.text().stripWhiteSpace().isEmpty()) {
+        editor.setIsGettingDictionaryPopupInfo(false);
         return popupInfo;
+    }
 
     RenderObject* renderer = range.startContainer().renderer();
     const RenderStyle& style = renderer->style();
 
     Vector<FloatQuad> quads;
     range.absoluteTextQuads(quads);
-    if (quads.isEmpty())
+    if (quads.isEmpty()) {
+        editor.setIsGettingDictionaryPopupInfo(false);
         return popupInfo;
+    }
 
     IntRect rangeRect = frame->view()->contentsToWindow(quads[0].enclosingBoundingBox());
 
@@ -540,6 +548,8 @@ static IntRect elementBoundingBoxInWindowCoordinatesFromNode(Node* node)
 
     if (auto textIndicator = TextIndicator::createWithRange(range, indicatorOptions, presentationTransition))
         popupInfo.textIndicator = textIndicator->data();
+
+    editor.setIsGettingDictionaryPopupInfo(false);
     return popupInfo;
 }
 
index 34d3939..8264670 100644 (file)
@@ -1,3 +1,20 @@
+2016-10-31  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [WK1] Candidate visibility should not update as a result of selection during a dictionary lookup
+        https://bugs.webkit.org/show_bug.cgi?id=164236
+        <rdar://problem/28747712>
+
+        Reviewed by Beth Dakin.
+
+        See WebCore ChangeLog for more detail. Removes m_isGettingDictionaryPopupInfo from the WebPage in favor of
+        keeping track of the same state in Editor, so that both the WK1 and WK2 cases can share the same codepath.
+
+        * WebProcess/WebPage/WebPage.cpp:
+        (WebKit::WebPage::didChangeSelection):
+        * WebProcess/WebPage/WebPage.h:
+        * WebProcess/WebPage/mac/WebPageMac.mm:
+        (WebKit::WebPage::dictionaryPopupInfoForRange):
+
 2016-10-31  Simon Fraser  <simon.fraser@apple.com>
 
         Implement IntersectionObserver
index c5bb68d..95a7a98 100644 (file)
@@ -4855,10 +4855,11 @@ static bool needsPlainTextQuirk(bool needsQuirks, const URL& url)
 
 void WebPage::didChangeSelection()
 {
+    Frame& frame = m_page->focusController().focusedOrMainFrame();
     // The act of getting Dictionary Popup info can make selection changes that we should not propagate to the UIProcess.
     // Specifically, if there is a caret selection, it will change to a range selection of the word around the caret. And
     // then it will change back.
-    if (m_isGettingDictionaryPopupInfo)
+    if (frame.editor().isGettingDictionaryPopupInfo())
         return;
 
     // Similarly, we don't want to propagate changes to the web process when inserting text asynchronously, since we will
@@ -4866,7 +4867,6 @@ void WebPage::didChangeSelection()
     if (m_isSelectingTextWhileInsertingAsynchronously)
         return;
 
-    Frame& frame = m_page->focusController().focusedOrMainFrame();
     FrameView* view = frame.view();
 
     // If there is a layout pending, we should avoid populating EditorState that require layout to be done or it will
index 9a1b6b8..74a2625 100644 (file)
@@ -1499,7 +1499,6 @@ private:
     bool m_mainFrameProgressCompleted;
     bool m_shouldDispatchFakeMouseMoveEvents;
     bool m_isEditorStateMissingPostLayoutData { false };
-    bool m_isGettingDictionaryPopupInfo { false };
     bool m_isSelectingTextWhileInsertingAsynchronously { false };
 
     enum class EditorStateIsContentEditable { No, Yes, Unset };
index 2df3f84..ca87e63 100644 (file)
@@ -59,6 +59,7 @@
 #import <WebCore/BackForwardController.h>
 #import <WebCore/DataDetection.h>
 #import <WebCore/DictionaryLookup.h>
+#import <WebCore/Editor.h>
 #import <WebCore/EventHandler.h>
 #import <WebCore/FocusController.h>
 #import <WebCore/FrameLoader.h>
@@ -437,19 +438,24 @@ void WebPage::performDictionaryLookupOfCurrentSelection()
 
 DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& range, NSDictionary **options, TextIndicatorPresentationTransition presentationTransition)
 {
-    TemporaryChange<bool> isGettingDictionaryPopupInfoChange { m_isGettingDictionaryPopupInfo, true };
+    Editor& editor = frame->editor();
+    editor.setIsGettingDictionaryPopupInfo(true);
 
     DictionaryPopupInfo dictionaryPopupInfo;
-    if (range.text().stripWhiteSpace().isEmpty())
+    if (range.text().stripWhiteSpace().isEmpty()) {
+        editor.setIsGettingDictionaryPopupInfo(false);
         return dictionaryPopupInfo;
+    }
     
     RenderObject* renderer = range.startContainer().renderer();
     const RenderStyle& style = renderer->style();
 
     Vector<FloatQuad> quads;
     range.absoluteTextQuads(quads);
-    if (quads.isEmpty())
+    if (quads.isEmpty()) {
+        editor.setIsGettingDictionaryPopupInfo(false);
         return dictionaryPopupInfo;
+    }
 
     IntRect rangeRect = frame->view()->contentsToWindow(quads[0].enclosingBoundingBox());
 
@@ -479,12 +485,15 @@ DictionaryPopupInfo WebPage::dictionaryPopupInfoForRange(Frame* frame, Range& ra
         indicatorOptions |= TextIndicatorOptionIncludeSnapshotWithSelectionHighlight;
 
     RefPtr<TextIndicator> textIndicator = TextIndicator::createWithRange(range, indicatorOptions, presentationTransition);
-    if (!textIndicator)
+    if (!textIndicator) {
+        editor.setIsGettingDictionaryPopupInfo(false);
         return dictionaryPopupInfo;
+    }
 
     dictionaryPopupInfo.textIndicator = textIndicator->data();
     dictionaryPopupInfo.attributedString = scaledNSAttributedString;
 
+    editor.setIsGettingDictionaryPopupInfo(false);
     return dictionaryPopupInfo;
 }