Attachment 362612 Details for Bug 194873 – Patch

[patch] Patch

bug-194873-20190221090026.patch (text/plain), 33.64 KB, created by Wenson Hsieh on 2019-02-21 09:00:27 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-02-21 09:00:27 PST

Size: 33.64 KB

patch

obsolete

>Subversion Revision: 241865
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 37054ea18d85d8f620101f6e29f150effa393cad..3c0ef04fc266e07c7500f7adab7527738d0c33ea 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,66 @@
>+2019-02-20  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Callout menu overlaps in-page controls when editing a comment in github.com's issue tracker
>+        https://bugs.webkit.org/show_bug.cgi?id=194873
>+        <rdar://problem/46701974>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        On the topic of supporting web-based rich text editors on iOS, one problematic area has always been handling
>+        conflicts between platform UI (i.e., the system callout menu) and in-page text editing controls. This issue
>+        comes up in websites that don't use the "hidden contenteditable" approach to rich text editing, but also show
>+        additional controls in a toolbar or contextual menu above the selection. In these cases, what often happens is
>+        that system controls overlap controls in the page.
>+
>+        Luckily, the iOS callout menu (i.e. the private UICalloutBar) is capable of presenting with a list of "evasion
>+        rects" to avoid; if the callout bar would normally intersect with one of these rects, then a different
>+        orientation that does not intersect with one of these rects is chosen instead. Currently, the only rect added
>+        here by UIKit when presenting the callout menu is the bounding rect of the on-screen keyboard, but after
>+        <rdar://problem/48128337>, we now have a generalized mechanism for offering additional evasion rects before
>+        UIKit presents the callout menu.
>+
>+        This patch adopts the mechanism introduced in <rdar://problem/48128337>, and introduces a heuristic for
>+        determining the approximate location of controls in the page which might overlap the callout menu. This
>+        heuristic works by hit-testing for clickable (but non-editable) nodes above the bounds of the selection, which
>+        are additionally not hit-tested by advancing outwards from any of the other edges of the selection bounds.
>+        Additionally, any hit-tested nodes whose bounding rects are very large (relative to the content view size) are
>+        ignored (this deals with scenarios where the body or a large container element has a click handler). We then add
>+        the bounding rects of each of the nodes that fit this criteria to the list of rects for UIKit to avoid when
>+        presenting the system callout menu.
>+
>+        The result is that WebKit will, by default, avoid overlapping anything that looks like controls in the page when
>+        showing a callout menu in editable content. In practice, this fixes overlapping controls on most websites that
>+        roll their own context menu or toolbar in their rich text editor.
>+
>+        Test: editing/selection/ios/avoid-showing-callout-menu-over-controls.html
>+
>+        * Platform/spi/ios/UIKitSPI.h:
>+        * Shared/WebPreferences.yaml:
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _smartCalloutBarPositioningEnabled]):
>+
>+        Gate this new behavior on a runtime-enabled internal feature flag.
>+
>+        * UIProcess/API/Cocoa/WKWebViewInternal.h:
>+        * UIProcess/WebPageProxy.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView requestAutocorrectionRectsForString:withCompletionHandler:]):
>+        (-[WKContentView requestRectsToEvadeForSelectionCommandsWithCompletionHandler:]):
>+        (-[WKContentView requestAutocorrectionContextWithCompletionHandler:]):
>+
>+        Drive-by: handle null completion handler arguments more gracefully, by raising an NSException and bailing before
>+        attempting to invoke a nil block.
>+
>+        * UIProcess/ios/WebPageProxyIOS.mm:
>+        (WebKit::WebPageProxy::requestEvasionRectsAboveSelection):
>+
>+        See above for more detail.
>+
>+        * WebProcess/WebPage/WebPage.h:
>+        * WebProcess/WebPage/WebPage.messages.in:
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::requestEvasionRectsAboveSelection):
>+
> 2019-02-21  Antoine Quint  <graouts@apple.com>
> 
>         Move UIWebTouchEventsGestureRecognizer.activeTouchesByIdentifier to SPI
>diff --git a/Source/WebKit/Platform/spi/ios/UIKitSPI.h b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>index 6d4c195d39d19851b3be5e07d557baf4d513e191..6d87410cee655c7c8282c72351de07b9098a082d 100644
>--- a/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>+++ b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>@@ -988,6 +988,7 @@ typedef NS_OPTIONS(NSUInteger, UIDragOperation)
> #endif
> 
> @interface UICalloutBar : UIView
>++ (UICalloutBar *)activeCalloutBar;
> + (void)fadeSharedCalloutBar;
> @end
> 
>diff --git a/Source/WebKit/Shared/WebPreferences.yaml b/Source/WebKit/Shared/WebPreferences.yaml
>index 74a122c5b77fc5d41a28891ca0a53df5dec9fafe..c4e88ff5d7705bdccd53759f98e83315306fd6f7 100644
>--- a/Source/WebKit/Shared/WebPreferences.yaml
>+++ b/Source/WebKit/Shared/WebPreferences.yaml
>@@ -1559,6 +1559,14 @@ MousemoveEventHandlingPreventsDefaultEnabled:
>   category: internal
>   condition: ENABLE(TOUCH_EVENTS)
> 
>+SmartCalloutBarPositioningEnabled:
>+  type: bool
>+  defaultValue: true
>+  humanReadableName: "Smart Callout Bar Positioning"
>+  humanReadableDescription: "Avoid interactible content when showing the callout bar"
>+  category: internal
>+  webcoreBinding: none
>+
> # Deprecated
> 
> ICECandidateFilteringEnabled:
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index 752a3a49bb388ca4f16d1b9f37ace03c178ce9ae..67704bdf518bc3cdfc1a35b39c99b9eb9757dd04 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -475,6 +475,11 @@ - (BOOL)_isRetainingActiveFocusedState
>     return _focusPreservationCount || _activeFocusedStateRetainCount;
> }
> 
>+- (BOOL)_smartCalloutBarPositioningEnabled
>+{
>+    return [_configuration preferences]->_preferences->smartCalloutBarPositioningEnabled();
>+}
>+
> #endif
> 
> #if USE(APPLE_INTERNAL_SDK)
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>index af16fc38be38409c13bdda2117f6e13d362c72a8..ae53e19616d1f79d891253e47709fa277076a29d 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebViewInternal.h
>@@ -168,6 +168,7 @@ struct PrintInfo;
> @property (nonatomic, readonly) UIEdgeInsets _computedObscuredInset;
> @property (nonatomic, readonly) UIEdgeInsets _computedUnobscuredSafeAreaInset;
> @property (nonatomic, readonly, getter=_isRetainingActiveFocusedState) BOOL _retainingActiveFocusedState;
>+@property (nonatomic, readonly) BOOL _smartCalloutBarPositioningEnabled;
> #endif
> 
> #if USE(APPLE_INTERNAL_SDK)
>diff --git a/Source/WebKit/UIProcess/WebPageProxy.h b/Source/WebKit/UIProcess/WebPageProxy.h
>index faaa5b721df5efff5b121a7126a09f0a1e84e8ba..a1937bef9aeaf285f21f4d63d3c1e673508010a5 100644
>--- a/Source/WebKit/UIProcess/WebPageProxy.h
>+++ b/Source/WebKit/UIProcess/WebPageProxy.h
>@@ -687,6 +687,7 @@ public:
>     void cancelAutoscroll();
>     void hardwareKeyboardAvailabilityChanged();
>     bool isScrollingOrZooming() const { return m_isScrollingOrZooming; }
>+    void requestEvasionRectsAboveSelection(CompletionHandler<void(const Vector<WebCore::FloatRect>&)>&&);
> #if ENABLE(DATA_INTERACTION)
>     void didHandleDragStartRequest(bool started);
>     void didHandleAdditionalDragItemsRequest(bool added);
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index b60da74cc87c896b36289a37bf1936d3156ccbe8..caae25f9a1e45640e0efaa244c6d0ecb32eb5480 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -3354,9 +3354,13 @@ - (const WebKit::WKAutoCorrectionData&)autocorrectionData
> // The completion handler can pass nil if input does not match the actual text preceding the insertion point.
> - (void)requestAutocorrectionRectsForString:(NSString *)input withCompletionHandler:(void (^)(UIWKAutocorrectionRects *rectsForInput))completionHandler
> {
>+    if (!completionHandler) {
>+        [NSException raise:NSInvalidArgumentException format:@"Expected a nonnull completion handler in %s.", __PRETTY_FUNCTION__];
>+        return;
>+    }
>+
>     if (!input || ![input length]) {
>-        if (completionHandler)
>-            completionHandler(nil);
>+        completionHandler(nil);
>         return;
>     }
> 
>@@ -3374,8 +3378,47 @@ - (void)requestAutocorrectionRectsForString:(NSString *)input withCompletionHand
>         view->_autocorrectionData.textFirstRect = firstRect;
>         view->_autocorrectionData.textLastRect = lastRect;
> 
>-        if (completion)
>-            completion(rects.size() ? [WKAutocorrectionRects autocorrectionRectsWithRects:firstRect lastRect:lastRect] : nil);
>+        completion(rects.size() ? [WKAutocorrectionRects autocorrectionRectsWithRects:firstRect lastRect:lastRect] : nil);
>+    });
>+}
>+
>+- (void)requestRectsToEvadeForSelectionCommandsWithCompletionHandler:(void(^)(NSArray<NSValue *> *rects))completionHandler
>+{
>+    if (!completionHandler) {
>+        [NSException raise:NSInvalidArgumentException format:@"Expected a nonnull completion handler in %s.", __PRETTY_FUNCTION__];
>+        return;
>+    }
>+
>+    if ([self _shouldSuppressSelectionCommands] || !_webView._smartCalloutBarPositioningEnabled || _webView._editable) {
>+        completionHandler(@[ ]);
>+        return;
>+    }
>+
>+    if (_focusedElementInformation.elementType != WebKit::InputType::ContentEditable && _focusedElementInformation.elementType != WebKit::InputType::TextArea) {
>+        completionHandler(@[ ]);
>+        return;
>+    }
>+
>+    // Give the page some time to present custom editing UI before attempting to detect and evade it.
>+    auto delayBeforeShowingCalloutBar = (0.25_s).nanoseconds();
>+    dispatch_after(dispatch_time(DISPATCH_TIME_NOW, delayBeforeShowingCalloutBar), dispatch_get_main_queue(), [completion = makeBlockPtr(completionHandler), weakSelf = WeakObjCPtr<WKContentView>(self)] () mutable {
>+        if (!weakSelf) {
>+            completion(@[ ]);
>+            return;
>+        }
>+
>+        auto strongSelf = weakSelf.get();
>+        if (!strongSelf->_page) {
>+            completion(@[ ]);
>+            return;
>+        }
>+
>+        strongSelf->_page->requestEvasionRectsAboveSelection([completion = WTFMove(completion)] (auto& rects) {
>+            auto rectsAsValues = adoptNS([[NSMutableArray alloc] initWithCapacity:rects.size()]);
>+            for (auto& floatRect : rects)
>+                [rectsAsValues addObject:[NSValue valueWithCGRect:floatRect]];
>+            completion(rectsAsValues.get());
>+        });
>     });
> }
> 
>@@ -3538,8 +3581,10 @@ - (void)applyAutocorrection:(NSString *)correction toString:(NSString *)input wi
> 
> - (void)requestAutocorrectionContextWithCompletionHandler:(void (^)(UIWKAutocorrectionContext *autocorrectionContext))completionHandler
> {
>-    if (!completionHandler)
>+    if (!completionHandler) {
>+        [NSException raise:NSInvalidArgumentException format:@"Expected a nonnull completion handler in %s.", __PRETTY_FUNCTION__];
>         return;
>+    }
> 
> #if USE(UIKIT_KEYBOARD_ADDITIONS)
>     if ([self _disableAutomaticKeyboardUI]) {
>diff --git a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>index e5006ede5b504228143716adc7a405838f343943..2bb1cc9f6ee3deb50d9752d8caf607fff790b0be 100644
>--- a/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>+++ b/Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
>@@ -29,6 +29,7 @@
> #if PLATFORM(IOS_FAMILY)
> 
> #import "APIUIClient.h"
>+#import "Connection.h"
> #import "DataReference.h"
> #import "EditingRange.h"
> #import "GlobalFindInPageState.h"
>@@ -1118,6 +1119,16 @@ void WebPageProxy::hardwareKeyboardAvailabilityChanged()
>     m_process->send(Messages::WebPage::HardwareKeyboardAvailabilityChanged(), m_pageID);
> }
> 
>+void WebPageProxy::requestEvasionRectsAboveSelection(CompletionHandler<void(const Vector<WebCore::FloatRect>&)>&& callback)
>+{
>+    if (!isValid()) {
>+        callback({ });
>+        return;
>+    }
>+
>+    m_process->connection()->sendWithAsyncReply(Messages::WebPage::RequestEvasionRectsAboveSelection(), WTFMove(callback), m_pageID);
>+}
>+
> #if ENABLE(DATA_INTERACTION)
> 
> void WebPageProxy::didHandleDragStartRequest(bool started)
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.h b/Source/WebKit/WebProcess/WebPage/WebPage.h
>index 1dd2bf7d4b5adf47efc9a16d60d72baf2cc49778..9e87c199e3efdf8e39f78b002d7c282a7f6714db 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.h
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.h
>@@ -664,6 +664,7 @@ public:
>     void storeSelectionForAccessibility(bool);
>     void startAutoscrollAtPosition(const WebCore::FloatPoint&);
>     void cancelAutoscroll();
>+    void requestEvasionRectsAboveSelection(CompletionHandler<void(const Vector<WebCore::FloatRect>&)>&&);
> 
>     void contentSizeCategoryDidChange(const String&);
> 
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>index 2d5b7b5a6bcd09307183f449fad16c27cd64b681..a52bb23b9b3a5aba5c054ca539f8c289e8dc9db2 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.messages.in
>@@ -80,6 +80,7 @@ messages -> WebPage LegacyReceiver {
>     SyncApplyAutocorrection(String correction, String originalText) -> (bool autocorrectionApplied) Delayed
>     RequestAutocorrectionContext(WebKit::CallbackID callbackID)
>     AutocorrectionContextSync() -> (struct WebKit::WebAutocorrectionContext context) Delayed
>+    RequestEvasionRectsAboveSelection() -> (Vector<WebCore::FloatRect> rects) Async
>     GetPositionInformation(struct WebKit::InteractionInformationRequest request) -> (struct WebKit::InteractionInformationAtPosition information) Delayed
>     RequestPositionInformation(struct WebKit::InteractionInformationRequest request)
>     StartInteractionWithElementAtPosition(WebCore::IntPoint point)
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index e15e79be15356d20d1b582f0d7cf4aab33ea9edf..63dad2203b810e1f456cacda65e185262927765c 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -1504,6 +1504,105 @@ void WebPage::cancelAutoscroll()
>     m_page->mainFrame().eventHandler().cancelSelectionAutoscroll();
> }
> 
>+void WebPage::requestEvasionRectsAboveSelection(CompletionHandler<void(const Vector<WebCore::FloatRect>&)>&& reply)
>+{
>+    auto& frame = m_page->focusController().focusedOrMainFrame();
>+    auto frameView = makeRefPtr(frame.view());
>+    if (!frameView) {
>+        reply({ });
>+        return;
>+    }
>+
>+    auto& selection = frame.selection().selection();
>+    if (selection.isNone()) {
>+        reply({ });
>+        return;
>+    }
>+
>+    auto selectedRange = selection.toNormalizedRange();
>+    if (!selectedRange) {
>+        reply({ });
>+        return;
>+    }
>+
>+    if (!m_focusedElement || !m_focusedElement->renderer() || m_focusedElement->renderer()->isTransparentOrFullyClippedRespectingParentFrames()) {
>+        reply({ });
>+        return;
>+    }
>+
>+    FloatRect selectionBoundsInRootViewCoordinates;
>+    if (selection.isRange())
>+        selectionBoundsInRootViewCoordinates = frameView->contentsToRootView(selectedRange->absoluteBoundingBox());
>+    else
>+        selectionBoundsInRootViewCoordinates = frameView->contentsToRootView(frame.selection().absoluteCaretBounds());
>+
>+    auto centerOfTargetBounds = selectionBoundsInRootViewCoordinates.center();
>+    WebCore::FloatPoint centerTopInRootViewCoordinates { centerOfTargetBounds.x(), selectionBoundsInRootViewCoordinates.y() };
>+
>+    auto clickableNonEditableNode = [&] (const WebCore::FloatPoint& locationInRootViewCoordinates) -> Node* {
>+        FloatPoint adjustedPoint;
>+        auto* hitNode = m_page->mainFrame().nodeRespondingToClickEvents(locationInRootViewCoordinates, adjustedPoint);
>+        if (!hitNode || is<HTMLBodyElement>(hitNode) || is<Document>(hitNode) || hitNode->hasEditableStyle())
>+            return nullptr;
>+
>+        return hitNode;
>+    };
>+
>+    float scaleFactor = pageScaleFactor();
>+    // This heuristic attempts to find a list of rects to avoid when showing the callout menu on iOS.
>+    // First, hit-test several points above the bounds of the selection rect in search of clickable nodes that are not editable.
>+    // Secondly, hit-test several points around the edges of the selection rect and exclude any nodes found in the first round of
>+    // hit-testing if these nodes are also reachable by moving outwards from the left, right, or bottom edges of the selection.
>+    // Additionally, exclude any hit-tested nodes that are either very large relative to the size of the root view, or completely
>+    // encompass the selection bounds. The resulting rects are the bounds of these hit-tested nodes in root view coordinates.
>+    HashSet<Ref<Node>> hitTestedNodes;
>+    Vector<WebCore::FloatRect> rectsToAvoidInRootViewCoordinates;
>+    const Vector<FloatPoint, 5> offsetsForHitTesting {{ -30, -51 }, { 31, -50 }, { -60, -34 }, { 60, -36 }, { 0, -20 }};
>+    for (auto offset : offsetsForHitTesting) {
>+        offset.scale(1 / scaleFactor);
>+        if (auto* hitNode = clickableNonEditableNode(centerTopInRootViewCoordinates + offset))
>+            hitTestedNodes.add(*hitNode);
>+    }
>+
>+    const float marginForHitTestingSurroundingNodes = 80 / scaleFactor;
>+    Vector<FloatPoint, 3> exclusionHitTestLocations {
>+        { selectionBoundsInRootViewCoordinates.x() - marginForHitTestingSurroundingNodes, centerOfTargetBounds.y() },
>+        { centerOfTargetBounds.x(), selectionBoundsInRootViewCoordinates.maxY() + marginForHitTestingSurroundingNodes },
>+        { selectionBoundsInRootViewCoordinates.maxX() + marginForHitTestingSurroundingNodes, centerOfTargetBounds.y() }
>+    };
>+
>+    for (auto& location : exclusionHitTestLocations) {
>+        if (auto* nodeToExclude = clickableNonEditableNode(location))
>+            hitTestedNodes.remove(*nodeToExclude);
>+    }
>+
>+    // If the element is too large, don't consider it to be a potential element in a context menu or toolbar on the page.
>+    const double factorOfContentArea = 0.75;
>+    auto contextMenuElementSizeLimit = factorOfContentArea * pageScaleFactor() * m_page->mainFrame().view()->unobscuredContentRect().size();
>+
>+    for (auto& node : hitTestedNodes) {
>+        RefPtr<Element> element;
>+        if (is<Element>(node))
>+            element = downcast<Element>(node.ptr());
>+        else
>+            element = node->parentElement();
>+
>+        if (!element)
>+            continue;
>+
>+        auto bounds = element->boundsInRootViewSpace();
>+        if (bounds.width() > contextMenuElementSizeLimit.width() || bounds.height() > contextMenuElementSizeLimit.height())
>+            continue;
>+
>+        if (bounds.contains(enclosingIntRect(selectionBoundsInRootViewCoordinates)))
>+            continue;
>+
>+        rectsToAvoidInRootViewCoordinates.append(WTFMove(bounds));
>+    }
>+
>+    reply(WTFMove(rectsToAvoidInRootViewCoordinates));
>+}
>+
> void WebPage::getRectsForGranularityWithSelectionOffset(uint32_t granularity, int32_t offset, CallbackID callbackID)
> {
>     Frame& frame = m_page->focusController().focusedOrMainFrame();
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 4f32b37f60af1e516528fe5da88f4c1dda354310..19e4baee659dd7166d32c8ce0b0a3fb93759ffcf 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-02-20  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Callout menu overlaps in-page controls when editing a comment in github.com's issue tracker
>+        https://bugs.webkit.org/show_bug.cgi?id=194873
>+        <rdar://problem/46701974>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a couple of UIScriptController methods to make callout menu testing on iOS easier (see below).
>+
>+        * DumpRenderTree/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::menuRect const):
>+        (WTR::UIScriptController::isShowingMenu const):
>+        * TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
>+        * TestRunnerShared/UIScriptContext/UIScriptController.cpp:
>+        (WTR::UIScriptController::menuRect const):
>+
>+        Add a function to query the bounds of the callout menu in content coordinates.
>+
>+        (WTR::UIScriptController::isShowingMenu const):
>+
>+        Add a function to query whether the callout menu is shown (i.e., has finished its appearance animation).
>+
>+        * TestRunnerShared/UIScriptContext/UIScriptController.h:
>+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
>+        * WebKitTestRunner/ios/UIScriptControllerIOS.mm:
>+        (WTR::UIScriptController::rectForMenuAction const):
>+        (WTR::UIScriptController::menuRect const):
>+        (WTR::UIScriptController::isShowingMenu const):
>+        (WTR::findViewInHierarchyOfType): Deleted.
>+
> 2019-02-21  Rob Buis  <rbuis@igalia.com>
> 
>         Update MIME type parser
>diff --git a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>index ffb4dea6a2c7e0b0bb56710ee433dd11c0971640..f408cca177cc43b77706b2cdf73546285d04abce 100644
>--- a/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>+++ b/Tools/DumpRenderTree/ios/UIScriptControllerIOS.mm
>@@ -337,6 +337,16 @@ JSObjectRef UIScriptController::rectForMenuAction(JSStringRef) const
>     return nullptr;
> }
> 
>+JSObjectRef UIScriptController::menuRect() const
>+{
>+    return nullptr;
>+}
>+
>+bool UIScriptController::isShowingMenu() const
>+{
>+    return false;
>+}
>+
> void UIScriptController::platformSetDidEndScrollingCallback()
> {
> }
>diff --git a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>index 3a9c3e29b9c122f47623c02ca1e8f36e29a6576a..5ec9191afb0f3075424e726a6939f2971f835114 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>+++ b/Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
>@@ -225,6 +225,8 @@ interface UIScriptController {
> 
>     attribute object didShowMenuCallback;
>     attribute object didHideMenuCallback;
>+    readonly attribute boolean isShowingMenu;
>+    readonly attribute object menuRect;
>     object rectForMenuAction(DOMString action);
> 
>     attribute object willBeginZoomingCallback;
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>index b9472d0fd17769eb8adfaf4bf3cbc796b3de8c60..612ea9c83bafdad402a6b066d4bb89a7c93681cb 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.cpp
>@@ -532,11 +532,21 @@ void UIScriptController::platformSetDidHideMenuCallback()
> {
> }
> 
>+JSObjectRef UIScriptController::menuRect() const
>+{
>+    return nullptr;
>+}
>+
> JSObjectRef UIScriptController::rectForMenuAction(JSStringRef) const
> {
>     return nullptr;
> }
> 
>+bool UIScriptController::isShowingMenu() const
>+{
>+    return false;
>+}
>+
> void UIScriptController::platformClearAllCallbacks()
> {
> }
>diff --git a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>index 5f335ed21399f18b024b1f678cbbd0be4db46e1d..ce3584170a9c48761150f74f9df5526bb6467fe7 100644
>--- a/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>+++ b/Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
>@@ -164,7 +164,9 @@ public:
>     void setDidShowMenuCallback(JSValueRef);
>     JSValueRef didShowMenuCallback() const;
> 
>+    bool isShowingMenu() const;
>     JSObjectRef rectForMenuAction(JSStringRef action) const;
>+    JSObjectRef menuRect() const;
> 
>     void setDidEndScrollingCallback(JSValueRef);
>     JSValueRef didEndScrollingCallback() const;
>diff --git a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>index 28d57ac2a66ceb10e6150e11b68da0c2c9ae7688..1d3a18c56a8a2028e5b471fa387146e24837fa84 100644
>--- a/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>+++ b/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
>@@ -56,6 +56,7 @@
> @property (nonatomic, assign) UIEdgeInsets overrideSafeAreaInsets;
> 
> @property (nonatomic, readonly, getter=isShowingKeyboard) BOOL showingKeyboard;
>+@property (nonatomic, readonly, getter=isShowingMenu) BOOL showingMenu;
> @property (nonatomic, assign) BOOL usesSafariLikeRotation;
> @property (nonatomic, readonly, getter=isInteractingWithFormControl) BOOL interactingWithFormControl;
> 
>diff --git a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>index 0987d2f27bd7d166c5d7fabdb045b8874e2af90c..7cc49766ebdc00aed788f129132d97800e5f54e5 100644
>--- a/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>+++ b/Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm
>@@ -117,19 +117,6 @@ static BOOL forEachViewInHierarchy(UIView *view, void(^mapFunction)(UIView *subv
>     return stop;
> }
> 
>-static UIView *findViewInHierarchyOfType(UIView *view, Class viewClass)
>-{
>-    __block RetainPtr<UIView> foundView;
>-    forEachViewInHierarchy(view, ^(UIView *subview, BOOL *stop) {
>-        if (![subview isKindOfClass:viewClass])
>-            return;
>-
>-        foundView = subview;
>-        *stop = YES;
>-    });
>-    return foundView.autorelease();
>-}
>-
> static NSArray<UIView *> *findAllViewsInHierarchyOfType(UIView *view, Class viewClass)
> {
>     __block RetainPtr<NSMutableArray> views = adoptNS([[NSMutableArray alloc] init]);
>@@ -883,25 +870,21 @@ JSObjectRef UIScriptController::rectForMenuAction(JSStringRef jsAction) const
> 
>     UIWindow *windowForButton = nil;
>     UIButton *buttonForAction = nil;
>-    for (UIWindow *window in UIApplication.sharedApplication.windows) {
>-        if (![window isKindOfClass:UITextEffectsWindow.class])
>-            continue;
>+    UIView *calloutBar = UICalloutBar.activeCalloutBar;
>+    if (!calloutBar.window)
>+        return nullptr;
> 
>-        UIView *calloutBar = findViewInHierarchyOfType(window, UICalloutBar.class);
>-        if (!calloutBar)
>+    for (UIButton *button in findAllViewsInHierarchyOfType(calloutBar, UIButton.class)) {
>+        NSString *buttonTitle = [button titleForState:UIControlStateNormal];
>+        if (!buttonTitle.length)
>             continue;
> 
>-        for (UIButton *button in findAllViewsInHierarchyOfType(calloutBar, UIButton.class)) {
>-            NSString *buttonTitle = [button titleForState:UIControlStateNormal];
>-            if (!buttonTitle.length)
>-                continue;
>-
>-            if (![buttonTitle isEqualToString:(__bridge NSString *)action.get()])
>-                continue;
>+        if (![buttonTitle isEqualToString:(__bridge NSString *)action.get()])
>+            continue;
> 
>-            buttonForAction = button;
>-            windowForButton = window;
>-        }
>+        buttonForAction = button;
>+        windowForButton = calloutBar.window;
>+        break;
>     }
> 
>     if (!buttonForAction)
>@@ -911,6 +894,21 @@ JSObjectRef UIScriptController::rectForMenuAction(JSStringRef jsAction) const
>     return m_context->objectFromRect(WebCore::FloatRect(rectInRootViewCoordinates.origin.x, rectInRootViewCoordinates.origin.y, rectInRootViewCoordinates.size.width, rectInRootViewCoordinates.size.height));
> }
> 
>+JSObjectRef UIScriptController::menuRect() const
>+{
>+    UIView *calloutBar = UICalloutBar.activeCalloutBar;
>+    if (!calloutBar.window)
>+        return nullptr;
>+
>+    CGRect rectInRootViewCoordinates = [calloutBar convertRect:calloutBar.bounds toView:platformContentView()];
>+    return m_context->objectFromRect(WebCore::FloatRect(rectInRootViewCoordinates.origin.x, rectInRootViewCoordinates.origin.y, rectInRootViewCoordinates.size.width, rectInRootViewCoordinates.size.height));
>+}
>+
>+bool UIScriptController::isShowingMenu() const
>+{
>+    return TestController::singleton().mainWebView()->platformView().showingMenu;
>+}
>+
> void UIScriptController::platformSetDidEndScrollingCallback()
> {
>     TestRunnerWKWebView *webView = TestController::singleton().mainWebView()->platformView();
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6c9d3410a20953d6da7cc04d11be0e08b4b424b9..b470741ea40f8e0e0b3dda508336f3fd49baea35 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-02-20  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Callout menu overlaps in-page controls when editing a comment in github.com's issue tracker
>+        https://bugs.webkit.org/show_bug.cgi?id=194873
>+        <rdar://problem/46701974>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a test to ensure that the we dodge clickable elements when showing the callout bar.
>+
>+        * editing/selection/ios/avoid-showing-callout-menu-over-controls-expected.txt: Added.
>+        * editing/selection/ios/avoid-showing-callout-menu-over-controls.html: Added.
>+        * resources/ui-helper.js:
>+        (window.UIHelper.waitForMenuToShow.return.new.Promise):
>+        (window.UIHelper.waitForMenuToShow):
>+        (window.UIHelper.menuRect):
>+        (window.UIHelper):
>+
> 2019-02-21  Diego Pino Garcia  <dpino@igalia.com>
> 
>         [GTK] Gardening for unexpected passes
>diff --git a/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls-expected.txt b/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..15d902b10e4cdeed8c1989e77a437337e10e113c
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls-expected.txt
>@@ -0,0 +1,12 @@
>+              
>+
>+Verifies that we don't show the system callout menu over controls. To test manually: focus the editable document in the iframe, select some text, and confirm that the callout bar is shown beneath the selection instead of above the selection.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS centerY(grabberRect) < centerY(menuRect) is true
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls.html b/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..be5d437df5e99e6f2e70dbd68dce9300864ea650
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/avoid-showing-callout-menu-over-controls.html
>@@ -0,0 +1,76 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+<meta name=viewport content="width=device-width, initial-scale=1">
>+<style>
>+body, html {
>+    width: 100%;
>+    height: 100%;
>+    margin: 0;
>+}
>+
>+iframe {
>+    width: 320px;
>+    height: 100px;
>+}
>+
>+#controls {
>+    margin-top: 20px;
>+    margin-bottom: 10px;
>+}
>+
>+input[type=button] {
>+    height: 1em;
>+}
>+</style>
>+<script>
>+jsTestIsAsync = true;
>+
>+async function runTest() {
>+    description("Verifies that we don't show the system callout menu over controls. To test manually: focus the editable document in the iframe, select some text, and confirm that the callout bar is shown beneath the selection instead of above the selection.");
>+
>+    centerX = rect => rect.left + rect.width / 2;
>+    centerY = rect => rect.top + rect.height / 2;
>+
>+    await UIHelper.activateAndWaitForInputSessionAt(160, 100);
>+
>+    const iframeDocument = document.querySelector("iframe").contentDocument;
>+    iframeDocument.getSelection().selectAllChildren(iframeDocument.body);
>+
>+    grabberRect = {};
>+    while (!grabberRect.width && !grabberRect.height)
>+        grabberRect = await UIHelper.getSelectionEndGrabberViewRect();
>+
>+    await UIHelper.activateAt(centerX(grabberRect), centerY(grabberRect));
>+    await UIHelper.waitForMenuToShow();
>+    menuRect = await UIHelper.menuRect();
>+
>+    document.activeElement.blur();
>+
>+    shouldBe("centerY(grabberRect) < centerY(menuRect)", "true");
>+    finishJSTest();
>+}
>+
>+addEventListener("load", runTest);
>+</script>
>+</head>
>+<body>
>+<div id="controls">
>+    <input type="button" value="1"></input>
>+    <input type="button" value="2"></input>
>+    <input type="button" value="3"></input>
>+    <input type="button" value="4"></input>
>+    <input type="button" value="5"></input>
>+    <input type="button" value="6"></input>
>+    <input type="button" value="7"></input>
>+    <input type="button" value="8"></input>
>+</div>
>+<div>
>+    <iframe srcdoc="<body contenteditable>Lorem ipsum dolor sit amet, consectetur adipisicing elit, sed do eiusmod.</body>"></iframe>
>+</div>
>+<p id="description"></p>
>+<p id="console"></p>
>+</body>
>+</html>
>\ No newline at end of file
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 4de0d14ea2a82877419b0119d44a5e7514076a8a..9e5fcbe0df6002a2d337170d992b5fcfc009dba6 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -665,4 +665,24 @@ window.UIHelper = class UIHelper {
>         const script = "JSON.stringify([uiController.lastUndoLabel, uiController.firstRedoLabel])";
>         return new Promise(resolve => testRunner.runUIScript(script, result => resolve(JSON.parse(result))));
>     }
>+
>+    static waitForMenuToShow()
>+    {
>+        return new Promise(resolve => {
>+            testRunner.runUIScript(`
>+                (function() {
>+                    if (!uiController.isShowingMenu)
>+                        uiController.didShowMenuCallback = () => uiController.uiScriptComplete();
>+                    else
>+                        uiController.uiScriptComplete();
>+                })()`, resolve);
>+        });
>+    }
>+
>+    static menuRect()
>+    {
>+        return new Promise(resolve => {
>+            testRunner.runUIScript("JSON.stringify(uiController.menuRect)", result => resolve(JSON.parse(result)));
>+        });
>+    }
> }

Flags:

thorton: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194873: 362590 | 362592 | 362612 | 362769