Title: [284766] trunk
Revision
284766
Author
[email protected]
Date
2021-10-24 13:39:35 -0700 (Sun, 24 Oct 2021)

Log Message

REGRESSION (iOS 15): Safari shows zoom callout even if -webkit-user-select is none
https://bugs.webkit.org/show_bug.cgi?id=231161
rdar://83863266

Reviewed by Darin Adler.

Source/WebKit:

Make several minor tweaks to prevent the text interaction assistant's loupe gesture from beginning when long
pressing inside content with `-webkit-user-select: none;`. Importantly, this prevents both the text
interaction's haptic feedback and the text selection magnifier UI (introduced in iOS 15) from showing up. See
comments below for more details.

Test: editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html

* Shared/ios/InteractionInformationAtPosition.h:
(WebKit::InteractionInformationAtPosition::isSelectable const):

Add a helper method to return whether the `selectability` flag is equal to `Selectable`, and use this in places
where we current check the `isSelectable` flag.

* Shared/ios/InteractionInformationAtPosition.mm:
(WebKit::InteractionInformationAtPosition::encode const):
(WebKit::InteractionInformationAtPosition::decode):

Break the current `isSelectable` flag out into different enum types, which enumerate the reasons why we might
need to treat hit-tested content as non-user-selectable. Importantly, this allows us to only early return inside
`-textInteractionGesture:shouldBeginAtPoint:` below if the element has an explicit `-webkit-user-select: none;`,
and not because of the other reasons (i.e. large element bounds or the fact that we're long pressing editable
text while not editing).

This nuance is important in order to continue allowing the loupe gesture (which manifests as a floating caret)
to begin when long pressing inside a focused a text field.

* UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView hasSelectablePositionAtPoint:]):
(-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):

Return NO here in the case where we're recognizing a loupe gesture (i.e. long press) inside content with
`-webkit-user-select: none;` (by consulting the new `selectability` enumeration). This allows us to prevent both
haptic feedback as well as the new magnifier UI from triggering when long pressing inside content that has
explicitly disabled text selection.

(-[WKContentView closestPositionToPoint:]):
* WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::selectionPositionInformation):
(WebKit::WebPage::positionInformation):

Additionally populate the `selectability` flag even when long pressing inside images and links. Instead of
putting the call to `selectionPositionInformation` behind the `isLink`/`isImage` check, move that condition into
`selectionPositionInformation` in the form of an early return, and always populate `selectability` in either
case.

LayoutTests:

See Source/WebKit/ChangeLog for more details.

* editing/selection/ios/do-not-allow-text-selection-in-user-select-none-expected.txt: Added.
* editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (284765 => 284766)


--- trunk/LayoutTests/ChangeLog	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/LayoutTests/ChangeLog	2021-10-24 20:39:35 UTC (rev 284766)
@@ -1,3 +1,16 @@
+2021-10-24  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (iOS 15): Safari shows zoom callout even if -webkit-user-select is none
+        https://bugs.webkit.org/show_bug.cgi?id=231161
+        rdar://83863266
+
+        Reviewed by Darin Adler.
+
+        See Source/WebKit/ChangeLog for more details.
+
+        * editing/selection/ios/do-not-allow-text-selection-in-user-select-none-expected.txt: Added.
+        * editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html: Added.
+
 2021-10-24  Alexey Shvayka  <[email protected]>
 
         document.open() and friends use incorrect document as a source for reseted document's URL

Added: trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none-expected.txt (0 => 284766)


--- trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none-expected.txt	2021-10-24 20:39:35 UTC (rev 284766)
@@ -0,0 +1,12 @@
+This test verifies that long pressing an element with -webkit-user-select: none; does not begin a text selection gesture. To manually run the test, long press the square below and verify that no text is selected.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS getSelection().toString() is ""
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Hello world
+
+

Added: trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html (0 => 284766)


--- trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html	                        (rev 0)
+++ trunk/LayoutTests/editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html	2021-10-24 20:39:35 UTC (rev 284766)
@@ -0,0 +1,36 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+html, body {
+    font-size: 16px;
+}
+
+#target {
+    width: 150px;
+    height: 150px;
+    background-color: tomato;
+    -webkit-user-select: none;
+}
+</style>
+<script src=""
+<script src=""
+<script>
+jsTestIsAsync = true;
+description("This test verifies that long pressing an element with <code>-webkit-user-select: none;</code> does not begin a text selection gesture. To manually run the test, long press the square below and verify that no text is selected.");
+
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.longPressElement(document.getElementById("target"));
+    shouldBeEqualToString("getSelection().toString()", "");
+    finishJSTest();
+});
+</script>
+</head>
+<body>
+<p>Hello world</p>
+<div id="target"></div>
+</body>
+</html>
\ No newline at end of file

Modified: trunk/Source/WebKit/ChangeLog (284765 => 284766)


--- trunk/Source/WebKit/ChangeLog	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/Source/WebKit/ChangeLog	2021-10-24 20:39:35 UTC (rev 284766)
@@ -1,3 +1,56 @@
+2021-10-24  Wenson Hsieh  <[email protected]>
+
+        REGRESSION (iOS 15): Safari shows zoom callout even if -webkit-user-select is none
+        https://bugs.webkit.org/show_bug.cgi?id=231161
+        rdar://83863266
+
+        Reviewed by Darin Adler.
+
+        Make several minor tweaks to prevent the text interaction assistant's loupe gesture from beginning when long
+        pressing inside content with `-webkit-user-select: none;`. Importantly, this prevents both the text
+        interaction's haptic feedback and the text selection magnifier UI (introduced in iOS 15) from showing up. See
+        comments below for more details.
+
+        Test: editing/selection/ios/do-not-allow-text-selection-in-user-select-none.html
+
+        * Shared/ios/InteractionInformationAtPosition.h:
+        (WebKit::InteractionInformationAtPosition::isSelectable const):
+
+        Add a helper method to return whether the `selectability` flag is equal to `Selectable`, and use this in places
+        where we current check the `isSelectable` flag.
+
+        * Shared/ios/InteractionInformationAtPosition.mm:
+        (WebKit::InteractionInformationAtPosition::encode const):
+        (WebKit::InteractionInformationAtPosition::decode):
+
+        Break the current `isSelectable` flag out into different enum types, which enumerate the reasons why we might
+        need to treat hit-tested content as non-user-selectable. Importantly, this allows us to only early return inside
+        `-textInteractionGesture:shouldBeginAtPoint:` below if the element has an explicit `-webkit-user-select: none;`,
+        and not because of the other reasons (i.e. large element bounds or the fact that we're long pressing editable
+        text while not editing).
+
+        This nuance is important in order to continue allowing the loupe gesture (which manifests as a floating caret)
+        to begin when long pressing inside a focused a text field.
+
+        * UIProcess/ios/WKContentViewInteraction.mm:
+        (-[WKContentView hasSelectablePositionAtPoint:]):
+        (-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
+
+        Return NO here in the case where we're recognizing a loupe gesture (i.e. long press) inside content with
+        `-webkit-user-select: none;` (by consulting the new `selectability` enumeration). This allows us to prevent both
+        haptic feedback as well as the new magnifier UI from triggering when long pressing inside content that has
+        explicitly disabled text selection.
+
+        (-[WKContentView closestPositionToPoint:]):
+        * WebProcess/WebPage/ios/WebPageIOS.mm:
+        (WebKit::selectionPositionInformation):
+        (WebKit::WebPage::positionInformation):
+
+        Additionally populate the `selectability` flag even when long pressing inside images and links. Instead of
+        putting the call to `selectionPositionInformation` behind the `isLink`/`isImage` check, move that condition into
+        `selectionPositionInformation` in the form of an early return, and always populate `selectability` in either
+        case.
+
 2021-10-24  Darin Adler  <[email protected]>
 
         [Cocoa] Adopt bridge_cast and makeVector in a few more places, including cases where adoptCF/NS was used incorrectly

Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h (284765 => 284766)


--- trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.h	2021-10-24 20:39:35 UTC (rev 284766)
@@ -53,7 +53,15 @@
 
     bool canBeValid { true };
     std::optional<bool> nodeAtPositionHasDoubleClickHandler;
-    bool isSelectable { false };
+
+    enum class Selectability : uint8_t {
+        Selectable,
+        UnselectableDueToFocusableElement,
+        UnselectableDueToLargeElementBounds,
+        UnselectableDueToUserSelectNone,
+    };
+    Selectability selectability { Selectability::Selectable };
+
     bool isSelected { false };
     bool prefersDraggingOverTextSelection { false };
     bool isNearMarkedText { false };
@@ -107,10 +115,26 @@
     // we can fetch the cheap information and copy the snapshots into the new response).
     void mergeCompatibleOptionalInformation(const InteractionInformationAtPosition& oldInformation);
 
+    bool isSelectable() const { return selectability == Selectability::Selectable; }
+
     void encode(IPC::Encoder&) const;
     static WARN_UNUSED_RETURN bool decode(IPC::Decoder&, InteractionInformationAtPosition&);
 };
 
-}
+} // namespace WebKit
 
+namespace WTF {
+
+template<> struct EnumTraits<WebKit::InteractionInformationAtPosition::Selectability> {
+    using values = EnumValues<
+        WebKit::InteractionInformationAtPosition::Selectability,
+        WebKit::InteractionInformationAtPosition::Selectability::Selectable,
+        WebKit::InteractionInformationAtPosition::Selectability::UnselectableDueToFocusableElement,
+        WebKit::InteractionInformationAtPosition::Selectability::UnselectableDueToLargeElementBounds,
+        WebKit::InteractionInformationAtPosition::Selectability::UnselectableDueToUserSelectNone
+    >;
+};
+
+} // namespace WTF
+
 #endif // PLATFORM(IOS_FAMILY)

Modified: trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm (284765 => 284766)


--- trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/Source/WebKit/Shared/ios/InteractionInformationAtPosition.mm	2021-10-24 20:39:35 UTC (rev 284766)
@@ -40,7 +40,7 @@
 
     encoder << canBeValid;
     encoder << nodeAtPositionHasDoubleClickHandler;
-    encoder << isSelectable;
+    encoder << selectability;
     encoder << isSelected;
     encoder << prefersDraggingOverTextSelection;
     encoder << isNearMarkedText;
@@ -99,7 +99,7 @@
     if (!decoder.decode(result.nodeAtPositionHasDoubleClickHandler))
         return false;
 
-    if (!decoder.decode(result.isSelectable))
+    if (!decoder.decode(result.selectability))
         return false;
 
     if (!decoder.decode(result.isSelected))

Modified: trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (284765 => 284766)


--- trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm	2021-10-24 20:39:35 UTC (rev 284766)
@@ -2886,7 +2886,7 @@
         return YES;
 #endif
 
-    return _positionInformation.isSelectable;
+    return _positionInformation.isSelectable();
 }
 
 - (BOOL)pointIsNearMarkedText:(CGPoint)point
@@ -2954,6 +2954,9 @@
     if (![self ensurePositionInformationIsUpToDate:request])
         return NO;
 
+    if (gesture == UIWKGestureLoupe && _positionInformation.selectability == WebKit::InteractionInformationAtPosition::Selectability::UnselectableDueToUserSelectNone)
+        return NO;
+
 #if ENABLE(DATALIST_ELEMENT)
     if (_positionInformation.preventTextInteraction)
         return NO;
@@ -5254,7 +5257,7 @@
 #if PLATFORM(MACCATALYST)
     WebKit::InteractionInformationRequest request(WebCore::roundedIntPoint(point));
     [self requestAsynchronousPositionInformationUpdate:request];
-    if ([self _currentPositionInformationIsApproximatelyValidForRequest:request radiusForApproximation:2] && _positionInformation.isSelectable)
+    if ([self _currentPositionInformationIsApproximatelyValidForRequest:request radiusForApproximation:2] && _positionInformation.isSelectable())
         return [WKTextPosition textPositionWithRect:_positionInformation.caretRect];
 #endif
     return nil;

Modified: trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm (284765 => 284766)


--- trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-10-24 18:24:01 UTC (rev 284765)
+++ trunk/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm	2021-10-24 20:39:35 UTC (rev 284766)
@@ -2888,10 +2888,31 @@
     if (!hitNode || !hitNode->renderer())
         return;
 
-    RenderObject* renderer = hitNode->renderer();
-    boundsPositionInformation(*renderer, info);
+    auto* renderer = hitNode->renderer();
 
+    info.selectability = ([&] {
+        if (renderer->style().userSelectIncludingInert() == UserSelect::None)
+            return InteractionInformationAtPosition::Selectability::UnselectableDueToUserSelectNone;
+
+        if (is<Element>(*hitNode)) {
+            if (isAssistableElement(downcast<Element>(*hitNode)))
+                return InteractionInformationAtPosition::Selectability::UnselectableDueToFocusableElement;
+
+            if (rectIsTooBigForSelection(info.bounds, *result.innerNodeFrame())) {
+                // We don't want to select blocks that are larger than 97% of the visible area of the document.
+                // FIXME: Is this heuristic still needed, now that block selection has been removed?
+                return InteractionInformationAtPosition::Selectability::UnselectableDueToLargeElementBounds;
+            }
+        }
+
+        return InteractionInformationAtPosition::Selectability::Selectable;
+    })();
     info.isSelected = result.isSelected();
+
+    if (info.isLink || info.isImage)
+        return;
+
+    boundsPositionInformation(*renderer, info);
     
     if (is<Element>(*hitNode)) {
         Element& element = downcast<Element>(*hitNode);
@@ -2905,13 +2926,8 @@
         linkIndicatorPositionInformation(page, attachment, request, info);
         if (attachment.file())
             info.url = ""
-    } else {
-        info.isSelectable = renderer->style().userSelectIncludingInert() != UserSelect::None;
-        // We don't want to select blocks that are larger than 97% of the visible area of the document.
-        // FIXME: Is this heuristic still needed, now that block selection has been removed?
-        if (info.isSelectable && !hitNode->isTextNode())
-            info.isSelectable = !isAssistableElement(*downcast<Element>(hitNode)) && !rectIsTooBigForSelection(info.bounds, *result.innerNodeFrame());
     }
+
     for (RefPtr currentNode = hitNode; currentNode; currentNode = currentNode->parentOrShadowHostNode()) {
         auto* renderer = currentNode->renderer();
         if (!renderer)
@@ -3087,8 +3103,7 @@
     if (!info.isImage && request.includeImageData && is<HTMLImageElement>(hitTestNode))
         imagePositionInformation(*this, downcast<HTMLImageElement>(*hitTestNode), request, info);
 
-    if (!(info.isLink || info.isImage))
-        selectionPositionInformation(*this, request, info);
+    selectionPositionInformation(*this, request, info);
 
     // Prevent the callout bar from showing when tapping on the datalist button.
 #if ENABLE(DATALIST_ELEMENT)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to