Attachment 372645 Details for Bug 199080 – Patch

[patch] Patch

bug-199080-20190621135010.patch (text/plain), 11.64 KB, created by Tim Horton on 2019-06-21 13:50:11 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Tim Horton

Created: 2019-06-21 13:50:11 PDT

Size: 11.64 KB

patch

obsolete

>Subversion Revision: 246688
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 4fd8815b76f82cd558d133795d75dd3e71569551..9091955396b873b68ec9c284a135f8be8832cefe 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,33 @@
>+2019-06-21  Tim Horton  <timothy_horton@apple.com>
>+
>+        WebKit context menu highlights include extra padding
>+        https://bugs.webkit.org/show_bug.cgi?id=199080
>+        <rdar://problem/51140126>
>+
>+        Reviewed by Dean Jackson.
>+
>+        * Shared/ios/InteractionInformationRequest.cpp:
>+        (WebKit::InteractionInformationRequest::encode const):
>+        (WebKit::InteractionInformationRequest::decode):
>+        (WebKit::InteractionInformationRequest::isValidForRequest):
>+        (WebKit::InteractionInformationRequest::isApproximatelyValidForRequest):
>+        * Shared/ios/InteractionInformationRequest.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView gestureRecognizerShouldBegin:]):
>+        (-[WKContentView positionInformationForActionSheetAssistant:]):
>+        (-[WKContentView updatePositionInformationForActionSheetAssistant:]):
>+        (-[WKContentView _contextMenuInteraction:configurationForMenuAtLocation:completion:]):
>+        (-[WKContentView _interactionShouldBeginFromPreviewItemController:forPosition:]):
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::linkIndicatorPositionInformation):
>+        Disable the margin going forward, but leave it if we're using the legacy
>+        API, because there is no way to add margin at the platform level with
>+        the legacy API.
>+
>+        In the future we should always have no margin in our indicator, and just
>+        apply it in whatever consumes the indicator, so that there is no
>+        disagreement between clients about the size of the margin.
>+
> 2019-06-21  Keith Rollin  <krollin@apple.com>
> 
>         Address reference to UIColor.groupTableViewBackgroundColor in WKPasswordView.mm
>diff --git a/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp b/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp
>index 7f1063afa16b7cda43cc82b4066a31622cf240be..3d704e6e6e6da38a8bc7dd4021af1e1b5343625b 100644
>--- a/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp
>+++ b/Source/WebKit/Shared/ios/InteractionInformationRequest.cpp
>@@ -38,6 +38,7 @@ void InteractionInformationRequest::encode(IPC::Encoder& encoder) const
>     encoder << point;
>     encoder << includeSnapshot;
>     encoder << includeLinkIndicator;
>+    encoder << linkIndicatorShouldHaveLegacyMargins;
>     encoder << readonly;
> }
> 
>@@ -52,6 +53,9 @@ bool InteractionInformationRequest::decode(IPC::Decoder& decoder, InteractionInf
>     if (!decoder.decode(result.includeLinkIndicator))
>         return false;
> 
>+    if (!decoder.decode(result.linkIndicatorShouldHaveLegacyMargins))
>+        return false;
>+
>     if (!decoder.decode(result.readonly))
>         return false;
> 
>@@ -69,6 +73,9 @@ bool InteractionInformationRequest::isValidForRequest(const InteractionInformati
>     if (other.includeLinkIndicator && !includeLinkIndicator)
>         return false;
> 
>+    if (other.linkIndicatorShouldHaveLegacyMargins != linkIndicatorShouldHaveLegacyMargins)
>+        return false;
>+
>     if (!other.readonly && readonly)
>         return false;
> 
>@@ -85,6 +92,9 @@ bool InteractionInformationRequest::isApproximatelyValidForRequest(const Interac
> 
>     if (!other.readonly && readonly)
>         return false;
>+
>+    if (other.linkIndicatorShouldHaveLegacyMargins != linkIndicatorShouldHaveLegacyMargins)
>+        return false;
>     
>     return (other.point - point).diagonalLengthSquared() <= 4;
> }
>diff --git a/Source/WebKit/Shared/ios/InteractionInformationRequest.h b/Source/WebKit/Shared/ios/InteractionInformationRequest.h
>index 15bbe0943848e31df4299b3e9ea1cc607af213ba..b318b40e31ae712aedb7140f91a20df3834df4bb 100644
>--- a/Source/WebKit/Shared/ios/InteractionInformationRequest.h
>+++ b/Source/WebKit/Shared/ios/InteractionInformationRequest.h
>@@ -42,6 +42,8 @@ struct InteractionInformationRequest {
>     bool includeSnapshot { false };
>     bool includeLinkIndicator { false };
> 
>+    bool linkIndicatorShouldHaveLegacyMargins { false };
>+
>     // FIXME: This readonly flag ought to be true by default, but there are a number of interactions (e.g. selection) that currently
>     // rely on the fact that interaction information requests additionally change the focused frame. We should explicitly turn the
>     // readonly bit off in these scenarios, and make sure that all other position information requests don't move focus around.
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>index e96dedb66a619cde9cfe94758ac43e6356064f43..05d702ca01c9f4f7cd5c93d8e3a1dbc1607c35b5 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>@@ -516,6 +516,8 @@ FOR_EACH_PRIVATE_WKCONTENTVIEW_ACTION(DECLARE_WKCONTENTVIEW_ACTION_FOR_WEB_VIEW)
> 
> - (void)_didStartProvisionalLoadForMainFrame;
> 
>+@property (nonatomic, readonly, getter=_shouldUseContextMenus) BOOL _shouldUseContextMenus;
>+
> @end
> 
> @interface WKContentView (WKTesting)
>@@ -540,7 +542,6 @@ FOR_EACH_PRIVATE_WKCONTENTVIEW_ACTION(DECLARE_WKCONTENTVIEW_ACTION_FOR_WEB_VIEW)
> #else
> @interface WKContentView (WKInteractionPreview) <UIPreviewItemDelegate>
> #endif
>-- (bool)_shouldUseContextMenus;
> - (void)_registerPreview;
> - (void)_unregisterPreview;
> @end
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index c05f1c82aa8827085ca18395e211f7b5e4580a90..c5b647074dc534a07b6e27a08cc875c5b3cc209d 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -770,7 +770,7 @@ - (void)setupInteraction
>     [_highlightLongPressGestureRecognizer setDelegate:self];
> 
> #if HAVE(LINK_PREVIEW)
>-    if (![self _shouldUseContextMenus]) {
>+    if (!self._shouldUseContextMenus) {
>         [self addGestureRecognizer:_highlightLongPressGestureRecognizer.get()];
>         [self _createAndConfigureLongPressGestureRecognizer];
>     }
>@@ -2066,6 +2066,7 @@ - (BOOL)gestureRecognizerShouldBegin:(UIGestureRecognizer *)gestureRecognizer
>         if (self.traitCollection.forceTouchCapability == UIForceTouchCapabilityAvailable) {
>             request.includeSnapshot = true;
>             request.includeLinkIndicator = true;
>+            request.linkIndicatorShouldHaveLegacyMargins = !self._shouldUseContextMenus;
>         }
> 
>         [self requestAsynchronousPositionInformationUpdate:request];
>@@ -6059,6 +6060,7 @@ - (Optional<WebKit::InteractionInformationAtPosition>)positionInformationForActi
>     WebKit::InteractionInformationRequest request(_positionInformation.request.point);
>     request.includeSnapshot = true;
>     request.includeLinkIndicator = assistant.needsLinkIndicator;
>+    request.linkIndicatorShouldHaveLegacyMargins = !self._shouldUseContextMenus;
>     if (![self ensurePositionInformationIsUpToDate:request])
>         return WTF::nullopt;
> 
>@@ -6071,6 +6073,7 @@ - (void)updatePositionInformationForActionSheetAssistant:(WKActionSheetAssistant
>     WebKit::InteractionInformationRequest request(_positionInformation.request.point);
>     request.includeSnapshot = true;
>     request.includeLinkIndicator = assistant.needsLinkIndicator;
>+    request.linkIndicatorShouldHaveLegacyMargins = !self._shouldUseContextMenus;
> 
>     [self requestAsynchronousPositionInformationUpdate:request];
> }
>@@ -6189,6 +6192,14 @@ - (void)actionSheetAssistant:(WKActionSheetAssistant *)assistant getAlternateURL
>         completion(nil, nil);
> }
> 
>+- (BOOL)_shouldUseContextMenus
>+{
>+#if HAVE(LINK_PREVIEW) && USE(UICONTEXTMENU)
>+    return linkedOnOrAfter(WebKit::SDKVersion::FirstThatHasUIContextMenuInteraction);
>+#endif
>+    return NO;
>+}
>+
> #if ENABLE(DRAG_SUPPORT)
> 
> static BOOL shouldEnableDragInteractionForPolicy(_WKDragInteractionPolicy policy)
>@@ -7367,7 +7378,7 @@ - (NSDictionary *)_contentsOfUserInterfaceItem:(NSString *)userInterfaceItem
> 
> #if HAVE(LINK_PREVIEW)
>     if ([userInterfaceItem isEqualToString:@"linkPreviewPopoverContents"]) {
>-        if ([self _shouldUseContextMenus])
>+        if (self._shouldUseContextMenus)
>             return @{ userInterfaceItem: @{ @"pageURL": WTF::userVisibleString(_positionInformation.url) } };
> 
>         NSString *url = [_previewItemController previewData][UIPreviewDataLink];
>@@ -7412,21 +7423,13 @@ static NSString *previewIdentifierForElementAction(_WKElementAction *action)
> 
> @implementation WKContentView (WKInteractionPreview)
> 
>-- (bool)_shouldUseContextMenus
>-{
>-#if USE(UICONTEXTMENU)
>-    return linkedOnOrAfter(WebKit::SDKVersion::FirstThatHasUIContextMenuInteraction);
>-#endif
>-    return false;
>-}
>-
> - (void)_registerPreview
> {
>     if (!_webView.allowsLinkPreview)
>         return;
> 
> #if USE(UICONTEXTMENU)
>-    if ([self _shouldUseContextMenus]) {
>+    if (self._shouldUseContextMenus) {
>         _contextMenuInteraction = adoptNS([[UIContextMenuInteraction alloc] initWithDelegate:self]);
>         _contextMenuHasRequestedLegacyData = NO;
>         [self addInteraction:_contextMenuInteraction.get()];
>@@ -7445,9 +7448,6 @@ - (void)_registerPreview
>     }
> #endif
> 
>-    if (!_webView.allowsLinkPreview)
>-        return;
>-
>     _previewItemController = adoptNS([[UIPreviewItemController alloc] initWithView:self]);
>     [_previewItemController setDelegate:self];
>     _previewGestureRecognizer = _previewItemController.get().presentationGestureRecognizer;
>@@ -7458,7 +7458,7 @@ - (void)_registerPreview
> - (void)_unregisterPreview
> {
> #if USE(UICONTEXTMENU)
>-    if ([self _shouldUseContextMenus]) {
>+    if (self._shouldUseContextMenus) {
>         if (!_contextMenuInteraction)
>             return;
> 
>@@ -7697,6 +7697,7 @@ - (void)_contextMenuInteraction:(UIContextMenuInteraction *)interaction configur
>     WebKit::InteractionInformationRequest request { WebCore::roundedIntPoint(position) };
>     request.includeSnapshot = true;
>     request.includeLinkIndicator = true;
>+    request.linkIndicatorShouldHaveLegacyMargins = !self._shouldUseContextMenus;
> 
>     [self doAfterPositionInformationUpdate:[weakSelf = WeakObjCPtr<WKContentView>(self), completion = makeBlockPtr(completion)] (WebKit::InteractionInformationAtPosition) {
>         auto strongSelf = weakSelf.get();
>@@ -8052,6 +8053,7 @@ - (BOOL)_interactionShouldBeginFromPreviewItemController:(UIPreviewItemControlle
>     WebKit::InteractionInformationRequest request(WebCore::roundedIntPoint(position));
>     request.includeSnapshot = true;
>     request.includeLinkIndicator = true;
>+    request.linkIndicatorShouldHaveLegacyMargins = !self._shouldUseContextMenus;
>     if (![self ensurePositionInformationIsUpToDate:request])
>         return NO;
>     if (!_positionInformation.isLink && !_positionInformation.isImage && !_positionInformation.isAttachment)
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index 5aaca4b702b9eb3154470876cd8e0de7046c43cf..ad17b6e73ac7f2ab603193fbefd6f9aa1275ccaf 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -2492,7 +2492,7 @@ static void linkIndicatorPositionInformation(WebPage& page, Element& element, El
> 
>     auto linkRange = rangeOfContents(linkElement);
>     float deviceScaleFactor = page.corePage()->deviceScaleFactor();
>-    const float marginInPoints = 4;
>+    const float marginInPoints = request.linkIndicatorShouldHaveLegacyMargins ? 4 : 0;
> 
>     auto textIndicator = TextIndicator::createWithRange(linkRange.get(),
>         TextIndicatorOptionTightlyFitContent | TextIndicatorOptionRespectTextColor | TextIndicatorOptionPaintBackgrounds |

Actions: View | Formatted Diff | Diff

Attachments on bug 199080: 372584 | 372645