Title: [177242] trunk/Source/WebKit2
Revision
177242
Author
[email protected]
Date
2014-12-12 15:16:04 -0800 (Fri, 12 Dec 2014)

Log Message

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.

Modified Paths

Diff

Modified: trunk/Source/WebKit2/ChangeLog (177241 => 177242)


--- trunk/Source/WebKit2/ChangeLog	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/ChangeLog	2014-12-12 23:16:04 UTC (rev 177242)
@@ -1,3 +1,72 @@
+2014-12-12  Timothy Horton  <[email protected]>
+
+        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  <[email protected]>
 
         Keep single RefCounter to track whether any pages in a context are process suppressible

Modified: trunk/Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/API/Cocoa/WKViewPrivate.h	2014-12-12 23:16:04 UTC (rev 177242)
@@ -136,7 +136,7 @@
 - (void)_handleClickInPreviewView:(NSView *)previewView URL:(NSURL *)url;
 - (BOOL)_shouldUseStandardQuickLookPreview;
 
-- (void)_dismissActionMenuPopovers;
+- (void)_dismissContentRelativeChildWindows;
 #endif
 
 @end

Modified: trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/API/mac/WKView.mm	2014-12-12 23:16:04 UTC (rev 177242)
@@ -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 @@
     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 @@
     [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 @@
         [NSEvent removeMonitor:_data->_flagsChangedEventMonitor];
         _data->_flagsChangedEventMonitor = nil;
 
-        if (getLULookupDefinitionModuleClass())
-            [getLULookupDefinitionModuleClass() hideDefinition];
-        [self _dismissActionMenuPopovers];
+        [self _dismissContentRelativeChildWindows];
     }
 
     _data->_page->setIntrinsicDeviceScaleFactor([self _intrinsicDeviceScaleFactor]);
@@ -4207,6 +4202,8 @@
         return;
     }
 
+    [self _dismissContentRelativeChildWindows];
+
     [self _ensureGestureController];
 
     _data->_gestureController->handleMagnificationGesture(event.magnification, [self convertPoint:event.locationInWindow fromView:nil]);
@@ -4219,6 +4216,8 @@
         return;
     }
 
+    [self _dismissContentRelativeChildWindows];
+
     [self _ensureGestureController];
 
     _data->_gestureController->handleSmartMagnificationGesture([self convertPoint:event.locationInWindow fromView:nil]);
@@ -4236,11 +4235,15 @@
 
 - (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 @@
     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

Modified: trunk/Source/WebKit2/UIProcess/PageClient.h (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/PageClient.h	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/PageClient.h	2014-12-12 23:16:04 UTC (rev 177242)
@@ -228,8 +228,7 @@
     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;

Modified: trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/WebPageProxy.cpp	2014-12-12 23:16:04 UTC (rev 177242)
@@ -2678,9 +2678,7 @@
     // 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 @@
 {
     m_uiClient->pageDidScroll(this);
 #if PLATFORM(MAC)
-    m_pageClient.dismissActionMenuPopovers();
-    dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
+    m_pageClient.dismissContentRelativeChildWindows();
 #endif
 }
 
@@ -4597,9 +4594,7 @@
 #endif
 
 #if PLATFORM(MAC)
-    dismissCorrectionPanel(ReasonForDismissingAlternativeTextIgnored);
-    m_pageClient.dismissDictionaryLookupPanel();
-    m_pageClient.dismissActionMenuPopovers();
+    m_pageClient.dismissContentRelativeChildWindows();
 #endif
 
     PageLoadState::Transaction transaction = m_pageLoadState.transaction();

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.h	2014-12-12 23:16:04 UTC (rev 177242)
@@ -139,8 +139,7 @@
     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);

Modified: trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/mac/PageClientImpl.mm	2014-12-12 23:16:04 UTC (rev 177242)
@@ -575,21 +575,11 @@
         [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);
+    [m_wkView _dismissContentRelativeChildWindows];
 }
 
-void PageClientImpl::dismissActionMenuPopovers()
-{
-    [m_wkView _dismissActionMenuPopovers];
-}
-
 void PageClientImpl::showCorrectionPanel(AlternativeTextType type, const FloatRect& boundingBoxOfReplacedString, const String& replacedString, const String& replacementString, const Vector<String>& alternativeReplacementStrings)
 {
 #if USE(AUTOCORRECTION_PANEL)
@@ -716,9 +706,7 @@
 
 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)

Modified: trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.h (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.h	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.h	2014-12-12 23:16:04 UTC (rev 177242)
@@ -59,7 +59,6 @@
     _WKActionMenuType _type;
     RetainPtr<NSSharingServicePicker> _sharingServicePicker;
 
-    BOOL _isShowingTextIndicator;
     BOOL _hasActivatedActionContext;
 
     RetainPtr<DDActionContext> _currentActionContext;
@@ -75,8 +74,6 @@
 
 - (void)didPerformActionMenuHitTest:(const WebKit::ActionMenuHitTestResult&)hitTestResult userData:(API::Object*)userData;
 
-- (void)dismissActionMenuPopovers;
-
 @end
 
 #endif // PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101000

Modified: trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.mm (177241 => 177242)


--- trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.mm	2014-12-12 23:12:04 UTC (rev 177241)
+++ trunk/Source/WebKit2/UIProcess/mac/WKActionMenuController.mm	2014-12-12 23:16:04 UTC (rev 177242)
@@ -59,8 +59,6 @@
 @interface WKActionMenuController () <NSSharingServiceDelegate, NSSharingServicePickerDelegate>
 - (void)_updateActionMenuItems;
 - (BOOL)_canAddMediaToPhotos;
-- (void)_showTextIndicator;
-- (void)_hideTextIndicator;
 - (void)_clearActionMenuState;
 @end
 
@@ -102,7 +100,7 @@
     if (menu != _wkView.actionMenu)
         return;
 
-    [self dismissActionMenuPopovers];
+    [_wkView _dismissContentRelativeChildWindows];
 
     _page->performActionMenuHitTestAtLocation([_wkView convertPoint:event.locationInWindow fromView:nil]);
 
@@ -181,37 +179,6 @@
     [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 @@
     _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]]];
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to