TextIndicator (and other view-relative subwindows) is not dismissed on zoom gesture
authortimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 23:16:04 +0000 (23:16 +0000)
committertimothy_horton@apple.com <timothy_horton@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 Dec 2014 23:16:04 +0000 (23:16 +0000)
https://bugs.webkit.org/show_bug.cgi?id=139580
<rdar://problem/19226350>

Reviewed by Anders Carlsson.

* UIProcess/PageClient.h:
* UIProcess/mac/PageClientImpl.h:
* UIProcess/mac/PageClientImpl.mm:
(WebKit::PageClientImpl::dismissContentRelativeChildWindows):
(WebKit::PageClientImpl::dismissDictionaryLookupPanel): Deleted.
(WebKit::PageClientImpl::dismissActionMenuPopovers): Deleted.
Merge dismissDictionaryLookupPanel and dismissActionMenuPopovers into
_dismissContentRelativeChildWindows, which also dismisses TextIndicator and the
text correction panel. Dismissal of any child windows which are anchored to
the page content now bottlenecks through this function.

(WebKit::PageClientImpl::navigationGestureDidBegin):
Dismiss all anchored subwindows when starting a navigation gesture, not
just TextIndicator and action menu popovers.

* UIProcess/API/mac/WKView.mm:
(-[WKView scrollWheel:]):
No need to dismiss TextIndicator here; we've got this covered in the more-inclusive
WebPageProxy::pageDidScroll, and will now dismiss more things.

(-[WKView mouseDown:]):
Dismiss more things.

(-[WKView viewDidMoveToWindow]):
Dismiss more things.

(-[WKView _dismissContentRelativeChildWindows]):
The aforementioned combined dismissal function.

(-[WKView magnifyWithEvent:]):
(-[WKView smartMagnifyWithEvent:]):
(-[WKView setMagnification:centeredAtPoint:]):
(-[WKView setMagnification:]):
Any time the page magnification changes, dismiss all the things.

* UIProcess/API/mac/WKViewPrivate.h:
Expose _dismissContentRelativeChildWindows instead of _dismissActionMenuPopovers;
for all existing clients, it makes more sense to dismiss all view-relative subwindows
than just a subset of them.

* UIProcess/WebPageProxy.cpp:
(WebKit::WebPageProxy::didCommitLoadForFrame):
(WebKit::WebPageProxy::pageDidScroll):
(WebKit::WebPageProxy::resetStateAfterProcessExited):
Make use of dismissAnchoredSubwindows.

* UIProcess/mac/WKActionMenuController.h:
* UIProcess/mac/WKActionMenuController.mm:
(-[WKActionMenuController prepareForMenu:withEvent:]):
Dismiss more things.

(-[WKActionMenuController _defaultMenuItemsForDataDetectedText]):
(-[WKActionMenuController dismissActionMenuPopovers]): Deleted.
dismissActionMenuPopovers is folded into WKView's _dismissContentRelativeChildWindows.

(-[WKActionMenuController _showTextIndicator]): Deleted.
(-[WKActionMenuController _hideTextIndicator]): Deleted.
Fold _showTextIndicator and _hideTextIndicator into their only callers.
_isShowingTextIndicator was never necessary, because it's OK to change
the current TextIndicator at any time, and a newer one is always more relevant.

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

Source/WebKit2/ChangeLog
Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h
Source/WebKit2/UIProcess/API/mac/WKView.mm
Source/WebKit2/UIProcess/PageClient.h
Source/WebKit2/UIProcess/WebPageProxy.cpp
Source/WebKit2/UIProcess/mac/PageClientImpl.h
Source/WebKit2/UIProcess/mac/PageClientImpl.mm
Source/WebKit2/UIProcess/mac/WKActionMenuController.h
Source/WebKit2/UIProcess/mac/WKActionMenuController.mm

index 2c5240e..6195118 100644 (file)
@@ -1,3 +1,72 @@
+2014-12-12  Timothy Horton  <timothy_horton@apple.com>
+
+        TextIndicator (and other view-relative subwindows) is not dismissed on zoom gesture
+        https://bugs.webkit.org/show_bug.cgi?id=139580
+        <rdar://problem/19226350>
+
+        Reviewed by Anders Carlsson.
+
+        * UIProcess/PageClient.h:
+        * UIProcess/mac/PageClientImpl.h:
+        * UIProcess/mac/PageClientImpl.mm:
+        (WebKit::PageClientImpl::dismissContentRelativeChildWindows):
+        (WebKit::PageClientImpl::dismissDictionaryLookupPanel): Deleted.
+        (WebKit::PageClientImpl::dismissActionMenuPopovers): Deleted.
+        Merge dismissDictionaryLookupPanel and dismissActionMenuPopovers into
+        _dismissContentRelativeChildWindows, which also dismisses TextIndicator and the
+        text correction panel. Dismissal of any child windows which are anchored to
+        the page content now bottlenecks through this function.
+
+        (WebKit::PageClientImpl::navigationGestureDidBegin):
+        Dismiss all anchored subwindows when starting a navigation gesture, not
+        just TextIndicator and action menu popovers.
+
+        * UIProcess/API/mac/WKView.mm:
+        (-[WKView scrollWheel:]):
+        No need to dismiss TextIndicator here; we've got this covered in the more-inclusive
+        WebPageProxy::pageDidScroll, and will now dismiss more things.
+
+        (-[WKView mouseDown:]):
+        Dismiss more things.
+
+        (-[WKView viewDidMoveToWindow]):
+        Dismiss more things.
+
+        (-[WKView _dismissContentRelativeChildWindows]):
+        The aforementioned combined dismissal function.
+
+        (-[WKView magnifyWithEvent:]):
+        (-[WKView smartMagnifyWithEvent:]):
+        (-[WKView setMagnification:centeredAtPoint:]):
+        (-[WKView setMagnification:]):
+        Any time the page magnification changes, dismiss all the things.
+
+        * UIProcess/API/mac/WKViewPrivate.h:
+        Expose _dismissContentRelativeChildWindows instead of _dismissActionMenuPopovers;
+        for all existing clients, it makes more sense to dismiss all view-relative subwindows
+        than just a subset of them.
+        
+        * UIProcess/WebPageProxy.cpp:
+        (WebKit::WebPageProxy::didCommitLoadForFrame):
+        (WebKit::WebPageProxy::pageDidScroll):
+        (WebKit::WebPageProxy::resetStateAfterProcessExited):
+        Make use of dismissAnchoredSubwindows.
+
+        * UIProcess/mac/WKActionMenuController.h:
+        * UIProcess/mac/WKActionMenuController.mm:
+        (-[WKActionMenuController prepareForMenu:withEvent:]):
+        Dismiss more things.
+
+        (-[WKActionMenuController _defaultMenuItemsForDataDetectedText]):
+        (-[WKActionMenuController dismissActionMenuPopovers]): Deleted.
+        dismissActionMenuPopovers is folded into WKView's _dismissContentRelativeChildWindows.
+
+        (-[WKActionMenuController _showTextIndicator]): Deleted.
+        (-[WKActionMenuController _hideTextIndicator]): Deleted.
+        Fold _showTextIndicator and _hideTextIndicator into their only callers.
+        _isShowingTextIndicator was never necessary, because it's OK to change
+        the current TextIndicator at any time, and a newer one is always more relevant.
+
 2014-12-12  Gavin Barraclough  <barraclough@apple.com>
 
         Keep single RefCounter to track whether any pages in a context are process suppressible
index 0572701..d7ded04 100644 (file)
 - (void)_handleClickInPreviewView:(NSView *)previewView URL:(NSURL *)url;
 - (BOOL)_shouldUseStandardQuickLookPreview;
 
-- (void)_dismissActionMenuPopovers;
+- (void)_dismissContentRelativeChildWindows;
 #endif
 
 @end
index 635a8e7..6030a9d 100644 (file)
@@ -79,6 +79,7 @@
 #import <QuartzCore/QuartzCore.h>
 #import <WebCore/AXObjectCache.h>
 #import <WebCore/ColorMac.h>
+#import <WebCore/DataDetectorsSPI.h>
 #import <WebCore/DragController.h>
 #import <WebCore/DragData.h>
 #import <WebCore/FloatRect.h>
@@ -1192,9 +1193,6 @@ NATIVE_MOUSE_EVENT_HANDLER(rightMouseUp)
     if (_data->_ignoresAllEvents)
         return;
 
-    // Work around <rdar://problem/19086993> by always clearing the active text indicator on scroll.
-    [self _setTextIndicator:nullptr fadeOut:NO];
-
     if (_data->_allowsBackForwardNavigationGestures) {
         [self _ensureGestureController];
         if (_data->_gestureController->handleScrollWheelEvent(event))
@@ -1243,8 +1241,7 @@ NATIVE_MOUSE_EVENT_HANDLER(rightMouseUp)
     [self _setMouseDownEvent:event];
     _data->_ignoringMouseDraggedEvents = NO;
 
-    // Work around <rdar://problem/19086993> by always clearing the active text indicator on mouseDown.
-    [self _setTextIndicator:nullptr fadeOut:NO];
+    [self _dismissContentRelativeChildWindows];
 #if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
     [_data->_actionMenuController wkView:self willHandleMouseDown:event];
 #endif
@@ -2580,9 +2577,7 @@ static void* keyValueObservingContext = &keyValueObservingContext;
         [NSEvent removeMonitor:_data->_flagsChangedEventMonitor];
         _data->_flagsChangedEventMonitor = nil;
 
-        if (getLULookupDefinitionModuleClass())
-            [getLULookupDefinitionModuleClass() hideDefinition];
-        [self _dismissActionMenuPopovers];
+        [self _dismissContentRelativeChildWindows];
     }
 
     _data->_page->setIntrinsicDeviceScaleFactor([self _intrinsicDeviceScaleFactor]);
@@ -4207,6 +4202,8 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
         return;
     }
 
+    [self _dismissContentRelativeChildWindows];
+
     [self _ensureGestureController];
 
     _data->_gestureController->handleMagnificationGesture(event.magnification, [self convertPoint:event.locationInWindow fromView:nil]);
@@ -4219,6 +4216,8 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
         return;
     }
 
+    [self _dismissContentRelativeChildWindows];
+
     [self _ensureGestureController];
 
     _data->_gestureController->handleSmartMagnificationGesture([self convertPoint:event.locationInWindow fromView:nil]);
@@ -4236,11 +4235,15 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
 
 - (void)setMagnification:(double)magnification centeredAtPoint:(NSPoint)point
 {
+    [self _dismissContentRelativeChildWindows];
+
     _data->_page->scalePageInViewCoordinates(magnification, roundedIntPoint(point));
 }
 
 - (void)setMagnification:(double)magnification
 {
+    [self _dismissContentRelativeChildWindows];
+
     FloatPoint viewCenter(NSMidX([self bounds]), NSMidY([self bounds]));
     _data->_page->scalePageInViewCoordinates(magnification, roundedIntPoint(viewCenter));
 }
@@ -4315,11 +4318,19 @@ static NSString *pathWithUniqueFilenameForPath(NSString *path)
     return nil;
 }
 
-- (void)_dismissActionMenuPopovers
+- (void)_dismissContentRelativeChildWindows
 {
-#if __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
-    [_data->_actionMenuController dismissActionMenuPopovers];
-#endif
+    // FIXME: We don't know which panel we are dismissing, it may not even be in the current page (see <rdar://problem/13875766>).
+    if (Class lookupDefinitionModuleClass = getLULookupDefinitionModuleClass())
+        [lookupDefinitionModuleClass hideDefinition];
+
+    DDActionsManager *actionsManager = [getDDActionsManagerClass() sharedManager];
+    if ([actionsManager respondsToSelector:@selector(requestBubbleClosureUnanchorOnFailure:)])
+        [actionsManager requestBubbleClosureUnanchorOnFailure:YES];
+
+    [self _setTextIndicator:nullptr fadeOut:NO];
+
+    static_cast<PageClient&>(*_data->_pageClient).dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
 }
 
 - (NSView *)_viewForPreviewingURL:(NSURL *)url initialFrameSize:(NSSize)initialFrameSize
index 3c41457..ab7988c 100644 (file)
@@ -228,8 +228,7 @@ public:
     virtual void pluginFocusOrWindowFocusChanged(uint64_t pluginComplexTextInputIdentifier, bool pluginHasFocusAndWindowHasFocus) = 0;
     virtual void setPluginComplexTextInputState(uint64_t pluginComplexTextInputIdentifier, PluginComplexTextInputState) = 0;
     virtual void didPerformDictionaryLookup(const DictionaryPopupInfo&) = 0;
-    virtual void dismissDictionaryLookupPanel() = 0;
-    virtual void dismissActionMenuPopovers() = 0;
+    virtual void dismissContentRelativeChildWindows() = 0;
     virtual void showCorrectionPanel(WebCore::AlternativeTextType, const WebCore::FloatRect& boundingBoxOfReplacedString, const String& replacedString, const String& replacementString, const Vector<String>& alternativeReplacementStrings) = 0;
     virtual void dismissCorrectionPanel(WebCore::ReasonForDismissingAlternativeText) = 0;
     virtual String dismissCorrectionPanelSoon(WebCore::ReasonForDismissingAlternativeText) = 0;
index d8b4d03..e3fa36c 100644 (file)
@@ -2678,9 +2678,7 @@ void WebPageProxy::didCommitLoadForFrame(uint64_t frameID, uint64_t navigationID
     // FIXME (bug 59111): didCommitLoadForFrame comes too late when restoring a page from b/f cache, making us disable secure event mode in password fields.
     // FIXME: A load going on in one frame shouldn't affect text editing in other frames on the page.
     m_pageClient.resetSecureInputState();
-    dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
-    m_pageClient.dismissDictionaryLookupPanel();
-    m_pageClient.dismissActionMenuPopovers();
+    m_pageClient.dismissContentRelativeChildWindows();
 #endif
 
     clearLoadDependentCallbacks();
@@ -3375,8 +3373,7 @@ void WebPageProxy::pageDidScroll()
 {
     m_uiClient->pageDidScroll(this);
 #if PLATFORM(MAC)
-    m_pageClient.dismissActionMenuPopovers();
-    dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
+    m_pageClient.dismissContentRelativeChildWindows();
 #endif
 }
 
@@ -4597,9 +4594,7 @@ void WebPageProxy::resetStateAfterProcessExited()
 #endif
 
 #if PLATFORM(MAC)
-    dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
-    m_pageClient.dismissDictionaryLookupPanel();
-    m_pageClient.dismissActionMenuPopovers();
+    m_pageClient.dismissContentRelativeChildWindows();
 #endif
 
     PageLoadState::Transaction transaction = m_pageLoadState.transaction();
index 32cc1d9..94d69a6 100644 (file)
@@ -139,8 +139,7 @@ private:
     virtual void makeFirstResponder();
     
     virtual void didPerformDictionaryLookup(const DictionaryPopupInfo&);
-    virtual void dismissDictionaryLookupPanel();
-    virtual void dismissActionMenuPopovers();
+    virtual void dismissContentRelativeChildWindows();
 
     virtual void showCorrectionPanel(WebCore::AlternativeTextType, const WebCore::FloatRect& boundingBoxOfReplacedString, const String& replacedString, const String& replacementString, const Vector<String>& alternativeReplacementStrings);
     virtual void dismissCorrectionPanel(WebCore::ReasonForDismissingAlternativeText);
index 8140330..c5bf491 100644 (file)
@@ -575,19 +575,9 @@ void PageClientImpl::didPerformDictionaryLookup(const DictionaryPopupInfo& dicti
         [getLULookupDefinitionModuleClass() showDefinitionForTerm:dictionaryPopupInfo.attributedString.string.get() atLocation:textBaselineOrigin options:mutableOptions.get()];
 }
 
-void PageClientImpl::dismissDictionaryLookupPanel()
+void PageClientImpl::dismissContentRelativeChildWindows()
 {
-    if (!getLULookupDefinitionModuleClass())
-        return;
-
-    // FIXME: We don't know which panel we are dismissing, it may not even be in the current page (see <rdar://problem/13875766>).
-    [getLULookupDefinitionModuleClass() hideDefinition];
-    setTextIndicator(nil, false);
-}
-
-void PageClientImpl::dismissActionMenuPopovers()
-{
-    [m_wkView _dismissActionMenuPopovers];
+    [m_wkView _dismissContentRelativeChildWindows];
 }
 
 void PageClientImpl::showCorrectionPanel(AlternativeTextType type, const FloatRect& boundingBoxOfReplacedString, const String& replacedString, const String& replacementString, const Vector<String>& alternativeReplacementStrings)
@@ -716,9 +706,7 @@ void PageClientImpl::beganExitFullScreen(const IntRect& initialFrame, const IntR
 
 void PageClientImpl::navigationGestureDidBegin()
 {
-    // Hide the text indicator and action menu popovers if they are visible.
-    setTextIndicator(nullptr, false);
-    dismissActionMenuPopovers();
+    dismissContentRelativeChildWindows();
 
 #if WK_API_ENABLED
     if (m_webView)
index 8299cab..67dac7b 100644 (file)
@@ -59,7 +59,6 @@ enum class ActionMenuState {
     _WKActionMenuType _type;
     RetainPtr<NSSharingServicePicker> _sharingServicePicker;
 
-    BOOL _isShowingTextIndicator;
     BOOL _hasActivatedActionContext;
 
     RetainPtr<DDActionContext> _currentActionContext;
@@ -75,8 +74,6 @@ enum class ActionMenuState {
 
 - (void)didPerformActionMenuHitTest:(const WebKit::ActionMenuHitTestResult&)hitTestResult userData:(API::Object*)userData;
 
-- (void)dismissActionMenuPopovers;
-
 @end
 
 #endif // PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000
index 8558e94..43d4def 100644 (file)
@@ -59,8 +59,6 @@ using namespace WebKit;
 @interface WKActionMenuController () <NSSharingServiceDelegate, NSSharingServicePickerDelegate>
 - (void)_updateActionMenuItems;
 - (BOOL)_canAddMediaToPhotos;
-- (void)_showTextIndicator;
-- (void)_hideTextIndicator;
 - (void)_clearActionMenuState;
 @end
 
@@ -102,7 +100,7 @@ using namespace WebKit;
     if (menu != _wkView.actionMenu)
         return;
 
-    [self dismissActionMenuPopovers];
+    [_wkView _dismissContentRelativeChildWindows];
 
     _page->performActionMenuHitTestAtLocation([_wkView convertPoint:event.locationInWindow fromView:nil]);
 
@@ -181,37 +179,6 @@ using namespace WebKit;
     [self _updateActionMenuItems];
 }
 
-- (void)dismissActionMenuPopovers
-{
-    DDActionsManager *actionsManager = [getDDActionsManagerClass() sharedManager];
-    if ([actionsManager respondsToSelector:@selector(requestBubbleClosureUnanchorOnFailure:)])
-        [actionsManager requestBubbleClosureUnanchorOnFailure:YES];
-
-    [self _hideTextIndicator];
-}
-
-#pragma mark Text Indicator
-
-- (void)_showTextIndicator
-{
-    if (_isShowingTextIndicator)
-        return;
-
-    if (_hitTestResult.detectedDataTextIndicator) {
-        _page->setTextIndicator(_hitTestResult.detectedDataTextIndicator->data(), false);
-        _isShowingTextIndicator = YES;
-    }
-}
-
-- (void)_hideTextIndicator
-{
-    if (!_isShowingTextIndicator)
-        return;
-
-    _page->clearTextIndicator();
-    _isShowingTextIndicator = NO;
-}
-
 #pragma mark Link actions
 
 - (NSArray *)_defaultMenuItemsForLink
@@ -422,11 +389,12 @@ static NSString *pathToPhotoOnDisk(NSString *suggestedFilename)
     _currentActionContext = [actionContext contextForView:_wkView altMode:YES interactionStartedHandler:^() {
         page->send(Messages::WebPage::DataDetectorsDidPresentUI(overlayID));
     } interactionChangedHandler:^() {
-        [self _showTextIndicator];
+        if (_hitTestResult.detectedDataTextIndicator)
+            _page->setTextIndicator(_hitTestResult.detectedDataTextIndicator->data(), false);
         page->send(Messages::WebPage::DataDetectorsDidChangeUI(overlayID));
     } interactionStoppedHandler:^() {
-        [self _hideTextIndicator];
         page->send(Messages::WebPage::DataDetectorsDidHideUI(overlayID));
+        page->clearTextIndicator();
     }];
 
     [_currentActionContext setHighlightFrame:[_wkView.window convertRectToScreen:[_wkView convertRect:_hitTestResult.detectedDataBoundingBox toView:nil]]];