Attachment 358019 Details for Bug 193005 – Patch

[patch] Patch

bug-193005-20181221221019.patch (text/plain), 19.33 KB, created by Wenson Hsieh on 2018-12-21 22:10:20 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2018-12-21 22:10:20 PST

Size: 19.33 KB

patch

obsolete

>Subversion Revision: 239515
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 7a618e95e5f09345611c32922256f03a2473a152..800c7acbd14aad99b220bd2d022d3b82e09636c2 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,55 @@
>+2018-12-21  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Suppress native selection behaviors when focusing a very small editable element
>+        https://bugs.webkit.org/show_bug.cgi?id=193005
>+        <rdar://problem/46583527>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        In r238146, I added a mechanism to detect when the selection is hidden within transparent editable elements, and
>+        used this to suppress native selection on iOS (such as selection handles, highlight, callout bar, etc.) to avoid
>+        conflicts between the page's editing UI and the platform.
>+
>+        However, one additional technique observed on some websites involves hiding the selection by moving it into a
>+        tiny (1x1) editable element. Here, we currently still present a callout bar with editing actions, as well as
>+        show a selection caret or handles on iOS. To fix this, we extend the mechanism added in r238146 by also
>+        suppressing the selection assistant in the case where the editable element's area is beneath a tiny minimum
>+        threshold.
>+
>+        Test: editing/selection/ios/hide-selection-in-tiny-contenteditable.html
>+
>+        * Shared/EditorState.cpp:
>+        (WebKit::EditorState::PostLayoutData::encode const):
>+        (WebKit::EditorState::PostLayoutData::decode):
>+        (WebKit::operator<<):
>+        * Shared/EditorState.h:
>+
>+        Rename selectionClipRect to focusedElementRect. We currently propagate the bounds of the focused element to the
>+        UI process through EditorState updates, but only for the purpose of returning it in the computed selection clip
>+        rect; instead, rename this member to something more general-purpose, so we can also use it when determining
>+        whether to suppress the selection assistant.
>+
>+        * UIProcess/API/Cocoa/WKWebView.mm:
>+        (-[WKWebView _candidateRect]):
>+        * UIProcess/Cocoa/WebViewImpl.mm:
>+        (WebKit::WebViewImpl::handleRequestedCandidates):
>+        * UIProcess/ios/WKContentViewInteraction.h:
>+
>+        Add a new SuppressSelectionAssistantReason that corresponds to focusing tiny editable elements.
>+
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView _zoomToRevealFocusedElement]):
>+        (-[WKContentView _selectionClipRect]):
>+        (-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:changingActivityState:userObject:]):
>+        (-[WKContentView _updateChangedSelection:]):
>+
>+        Check the size of the focused element, and begin or stop suppressing the selection assistant accordingly.
>+
>+        * WebProcess/WebPage/ios/WebPageIOS.mm:
>+        (WebKit::WebPage::platformEditorState const):
>+        * WebProcess/WebPage/mac/WebPageMac.mm:
>+        (WebKit::WebPage::platformEditorState const):
>+
> 2018-12-21  Keith Rollin  <krollin@apple.com>
> 
>         Crash in com.apple.WebKit: WebKit::WebResourceLoader::willSendRequest + 223
>diff --git a/Source/WebKit/Shared/EditorState.cpp b/Source/WebKit/Shared/EditorState.cpp
>index d319748c8da1818c99a58d6261baf6c9ab435b38..e30e939c02a7abccdfce67b0390ce719fb78b5d1 100644
>--- a/Source/WebKit/Shared/EditorState.cpp
>+++ b/Source/WebKit/Shared/EditorState.cpp
>@@ -112,7 +112,7 @@ void EditorState::PostLayoutData::encode(IPC::Encoder& encoder) const
>     encoder << caretRectAtStart;
> #endif
> #if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>-    encoder << selectionClipRect;
>+    encoder << focusedElementRect;
>     encoder << selectedTextLength;
>     encoder << textAlignment;
>     encoder << textColor;
>@@ -153,7 +153,7 @@ bool EditorState::PostLayoutData::decode(IPC::Decoder& decoder, PostLayoutData&
>         return false;
> #endif
> #if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>-    if (!decoder.decode(result.selectionClipRect))
>+    if (!decoder.decode(result.focusedElementRect))
>         return false;
>     if (!decoder.decode(result.selectedTextLength))
>         return false;
>@@ -262,8 +262,8 @@ TextStream& operator<<(TextStream& ts, const EditorState& editorState)
>         ts.dumpProperty("caretRectAtStart", editorState.postLayoutData().caretRectAtStart);
> #endif
> #if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>-    if (editorState.postLayoutData().selectionClipRect != IntRect())
>-        ts.dumpProperty("selectionClipRect", editorState.postLayoutData().selectionClipRect);
>+    if (editorState.postLayoutData().focusedElementRect != IntRect())
>+        ts.dumpProperty("focusedElementRect", editorState.postLayoutData().focusedElementRect);
>     if (editorState.postLayoutData().selectedTextLength)
>         ts.dumpProperty("selectedTextLength", editorState.postLayoutData().selectedTextLength);
>     if (editorState.postLayoutData().textAlignment != NoAlignment)
>diff --git a/Source/WebKit/Shared/EditorState.h b/Source/WebKit/Shared/EditorState.h
>index 780ecf7beebfa38ba61bd51cc63e3e99c79abff3..e314d0ae5a0150627a3b212a2c4e450e3d2d2920 100644
>--- a/Source/WebKit/Shared/EditorState.h
>+++ b/Source/WebKit/Shared/EditorState.h
>@@ -89,7 +89,7 @@ struct EditorState {
>         WebCore::IntRect caretRectAtStart;
> #endif
> #if PLATFORM(IOS_FAMILY) || PLATFORM(MAC)
>-        WebCore::IntRect selectionClipRect;
>+        WebCore::IntRect focusedElementRect;
>         uint64_t selectedTextLength { 0 };
>         uint32_t textAlignment { NoAlignment };
>         WebCore::Color textColor { WebCore::Color::black };
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index b2d33c44f4198128cec6fa70836c5f5b92c921b5..1468998341f305d0dbb02e03e2c373539529369e 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -6739,7 +6739,7 @@ - (void)_insertText:(id)string replacementRange:(NSRange)replacementRange
> 
> - (NSRect)_candidateRect
> {
>-    return _page->editorState().postLayoutData().selectionClipRect;
>+    return _page->editorState().postLayoutData().focusedElementRect;
> }
> 
> - (BOOL)_useSystemAppearance
>diff --git a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>index e6c872e4fade6cfc42bca333b747b821cf56e047..72bfd21ccd86a80c3ba54a3b4286f8e319045a6c 100644
>--- a/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>+++ b/Source/WebKit/UIProcess/Cocoa/WebViewImpl.mm
>@@ -3297,7 +3297,7 @@ void WebViewImpl::handleRequestedCandidates(NSInteger sequenceNumber, NSArray<NS
> 
> #if HAVE(TOUCH_BAR)
>     NSRange selectedRange = NSMakeRange(postLayoutData.candidateRequestStartPosition, postLayoutData.selectedTextLength);
>-    WebCore::IntRect offsetSelectionRect = postLayoutData.selectionClipRect;
>+    WebCore::IntRect offsetSelectionRect = postLayoutData.focusedElementRect;
>     offsetSelectionRect.move(0, offsetSelectionRect.height());
> 
>     [candidateListTouchBarItem() setCandidates:candidates forSelectedRange:selectedRange inString:postLayoutData.paragraphContextForCandidateRequest rect:offsetSelectionRect view:m_view.getAutoreleased() completionHandler:nil];
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>index c1f647402d9abe26168152c1e7101287d61bb199..d91a585ae247e637ccf138404acf8ba5dafc111c 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
>@@ -163,7 +163,8 @@ namespace WebKit {
> 
> enum SuppressSelectionAssistantReason : uint8_t {
>     FocusedElementIsTransparent = 1 << 0,
>-    DropAnimationIsRunning = 1 << 1
>+    FocusedElementIsTooSmall = 1 << 1,
>+    DropAnimationIsRunning = 1 << 2
> };
> 
> struct WKSelectionDrawingInfo {
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index f269395061f20a2ab9cf26a8e01bd5e7854c1c7a..502ac79259d41728ed913f5ff52eec8bf30f6e82 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -1398,7 +1398,7 @@ - (BOOL)_requiresKeyboardResetOnReload
> 
> - (void)_zoomToRevealFocusedElement
> {
>-    if (_suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTransparent))
>+    if (_suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTransparent) || _suppressSelectionAssistantReasons.contains(WebKit::FocusedElementIsTooSmall))
>         return;
> 
>     SetForScope<BOOL> isZoomingToRevealFocusedElementForScope { _isZoomingToRevealFocusedElement, YES };
>@@ -1465,7 +1465,7 @@ - (CGRect)_selectionClipRect
> {
>     if (!hasFocusedElement(_focusedElementInformation))
>         return CGRectNull;
>-    return _page->editorState().postLayoutData().selectionClipRect;
>+    return _page->editorState().postLayoutData().focusedElementRect;
> }
> 
> - (BOOL)gestureRecognizer:(UIGestureRecognizer *)preventingGestureRecognizer canPreventGestureRecognizer:(UIGestureRecognizer *)preventedGestureRecognizer
>@@ -4446,6 +4446,8 @@ static bool isAssistableInputType(WebKit::InputType type)
>     return false;
> }
> 
>+static const double minimumFocusedElementAreaForSuppressingSelectionAssistant = 4;
>+
> - (void)_elementDidFocus:(const WebKit::FocusedElementInformation&)information userIsInteracting:(BOOL)userIsInteracting blurPreviousNode:(BOOL)blurPreviousNode changingActivityState:(BOOL)changingActivityState userObject:(NSObject <NSSecureCoding> *)userObject
> {
>     SetForScope<BOOL> isChangingFocusForScope { _isChangingFocus, hasFocusedElement(_focusedElementInformation) };
>@@ -4474,6 +4476,11 @@ - (void)_elementDidFocus:(const WebKit::FocusedElementInformation&)information u
>     else
>         [self _stopSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTransparent];
> 
>+    if (information.elementRect.area() < minimumFocusedElementAreaForSuppressingSelectionAssistant)
>+        [self _beginSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTooSmall];
>+    else
>+        [self _stopSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTooSmall];
>+
>     switch (startInputSessionPolicy) {
>     case _WKFocusStartsInputSessionPolicyAuto:
>         // The default behavior is to allow node assistance if the user is interacting.
>@@ -5000,11 +5007,16 @@ - (void)_updateChangedSelection:(BOOL)force
>         return;
> 
>     auto& postLayoutData = state.postLayoutData();
>-    if (hasFocusedElement(_focusedElementInformation)) {
>+    if (!state.selectionIsNone && hasFocusedElement(_focusedElementInformation)) {
>         if (postLayoutData.elementIsTransparent)
>             [self _beginSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTransparent];
>         else
>             [self _stopSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTransparent];
>+
>+        if (postLayoutData.focusedElementRect.area() < minimumFocusedElementAreaForSuppressingSelectionAssistant)
>+            [self _beginSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTooSmall];
>+        else
>+            [self _stopSuppressingSelectionAssistantForReason:WebKit::FocusedElementIsTooSmall];
>     }
> 
>     WebKit::WKSelectionDrawingInfo selectionDrawingInfo(_page->editorState());
>diff --git a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>index f881fce68162899355af06773f4689675545117d..12d97a1fe4dad8d4b9f56c86d806c7c9b1f3f1ae 100644
>--- a/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>+++ b/Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
>@@ -241,7 +241,7 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
>     postLayoutData.insideFixedPosition = startNodeIsInsideFixedPosition || endNodeIsInsideFixedPosition;
>     if (!selection.isNone()) {
>         if (m_focusedElement && m_focusedElement->renderer()) {
>-            postLayoutData.selectionClipRect = view->contentsToRootView(m_focusedElement->renderer()->absoluteBoundingBoxRect());
>+            postLayoutData.focusedElementRect = view->contentsToRootView(m_focusedElement->renderer()->absoluteBoundingBoxRect());
>             postLayoutData.caretColor = m_focusedElement->renderer()->style().caretColor();
>             postLayoutData.elementIsTransparent = m_focusedElement->renderer()->isTransparentRespectingParentFrames();
>         }
>diff --git a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
>index fa7a6aaf1b60bda90dcd079ba1a9436f1216b0dc..e59b681e6a5b1ec2bb9b4cbad51e6f03f03e4d3a 100644
>--- a/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
>+++ b/Source/WebKit/WebProcess/WebPage/mac/WebPageMac.mm
>@@ -151,11 +151,11 @@ void WebPage::platformEditorState(Frame& frame, EditorState& result, IncludePost
>     Vector<FloatQuad> quads;
>     selectedRange->absoluteTextQuads(quads);
>     if (!quads.isEmpty())
>-        postLayoutData.selectionClipRect = frame.view()->contentsToWindow(quads[0].enclosingBoundingBox());
>+        postLayoutData.focusedElementRect = frame.view()->contentsToWindow(quads[0].enclosingBoundingBox());
>     else {
>         // Range::absoluteTextQuads() will be empty at the start of a paragraph.
>         if (selection.isCaret())
>-            postLayoutData.selectionClipRect = frame.view()->contentsToWindow(frame.selection().absoluteCaretBounds());
>+            postLayoutData.focusedElementRect = frame.view()->contentsToWindow(frame.selection().absoluteCaretBounds());
>     }
> }
> 
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ffd7c51aa49fa80a6c000d7b8a860ef60eadfc89..ed2d4f40219a2967593d4a78abc35d6b496e2808 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2018-12-21  Wenson Hsieh  <wenson_hsieh@apple.com>
>+
>+        [iOS] Suppress native selection behaviors when focusing a very small editable element
>+        https://bugs.webkit.org/show_bug.cgi?id=193005
>+        <rdar://problem/46583527>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add a new layout test to verify that native selection UI is suppressed when focusing a tiny (1px by 1px)
>+        editable element.
>+
>+        * editing/selection/ios/hide-selection-in-tiny-contenteditable-expected.txt: Added.
>+        * editing/selection/ios/hide-selection-in-tiny-contenteditable.html: Added.
>+        * resources/ui-helper.js:
>+        (window.UIHelper.zoomToScale):
>+
> 2018-12-21  Zalan Bujtas  <zalan@apple.com>
> 
>         Synchronous media query evaluation could destroy current Frame/FrameView.
>diff --git a/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable-expected.txt b/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..a4382c8240d5ff1255acc5886564034f13254f5e
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable-expected.txt
>@@ -0,0 +1,18 @@
>+Focus the editor
>+abcdefg
>+Verifies that selection UI is suppressed when the editable root is extremely small. To manually test, tap on the button above and verify that (1) the editable element is focused, and (2) selection handles are not shown.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+After focus, the caret rect is empty
>+After zooming in, the caret rect is empty
>+After making editor large, the caret rect is empty
>+After making editor opaque, the caret rect is (left=21, top=100, width=2, height=19)
>+After making editor tiny again, the caret rect is empty
>+After making editor transparent again, the caret rect is empty
>+After making editor large again, the caret rect is empty
>+After making editor opaque again, the caret rect is (left=50, top=100, width=2, height=19)
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable.html b/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..ee9c3777d79a795ccbb25260e73098d7812ca34c
>--- /dev/null
>+++ b/LayoutTests/editing/selection/ios/hide-selection-in-tiny-contenteditable.html
>@@ -0,0 +1,93 @@
>+<!DOCTYPE html> 
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width, initial-scale=1">
>+<script src="../../../resources/ui-helper.js"></script>
>+<script src="../../../resources/js-test.js"></script>
>+<style>
>+body, html {
>+    width: 100%;
>+    height: 100%;
>+    margin: 0;
>+}
>+
>+#editor {
>+    width: 1px;
>+    height: 1px;
>+    outline: none;
>+    overflow: hidden;
>+    opacity: 0;
>+}
>+
>+button {
>+    width: 320px;
>+    height: 100px;
>+}
>+</style>
>+</head>
>+<body>
>+<button onclick="editor.focus()">Focus the editor</button>
>+<div id="editor" contenteditable autocorrect="off" autocapitalize="off" spellcheck="false"></div>
>+<div id="description"></div>
>+<div id="console"></div>
>+<script>
>+jsTestIsAsync = true;
>+
>+function caretRectToString(rect) {
>+    if (!rect.width && !rect.height)
>+        return "empty";
>+    return `(left=${rect.left}, top=${rect.top}, width=${rect.width}, height=${rect.height})`;
>+}
>+
>+async function checkCaretRect(description)
>+{
>+    await UIHelper.ensurePresentationUpdate();
>+    const rect = await UIHelper.getUICaretViewRect();
>+    debug(`${description}, the caret rect is ${caretRectToString(rect)}`);
>+}
>+
>+(async () => {
>+    description("Verifies that selection UI is suppressed when the editable root is extremely small. To manually test, "
>+        + "tap on the button above and verify that (1) the editable element is focused, and (2) selection handles are "
>+        + "not shown.");
>+
>+    await UIHelper.activateAndWaitForInputSessionAt(160, 50);
>+    await checkCaretRect("After focus");
>+
>+    await UIHelper.zoomToScale(3);
>+    await UIHelper.typeCharacter("a");
>+    await checkCaretRect("After zooming in");
>+    await UIHelper.zoomToScale(1);
>+
>+    editor.style.width = "100px";
>+    editor.style.height = "100px";
>+    await UIHelper.typeCharacter("b");
>+    await checkCaretRect("After making editor large");
>+
>+    editor.style.opacity = 1;
>+    await UIHelper.typeCharacter("c");
>+    await checkCaretRect("After making editor opaque");
>+
>+    editor.style.width = "1px";
>+    editor.style.height = "1px";
>+    await UIHelper.typeCharacter("d");
>+    await checkCaretRect("After making editor tiny again");
>+
>+    editor.style.opacity = 0;
>+    await UIHelper.typeCharacter("e");
>+    await checkCaretRect("After making editor transparent again");
>+
>+    editor.style.width = "100px";
>+    editor.style.height = "100px";
>+    await UIHelper.typeCharacter("f");
>+    await checkCaretRect("After making editor large again");
>+
>+    editor.style.opacity = 1;
>+    await UIHelper.typeCharacter("g");
>+    await checkCaretRect("After making editor opaque again");
>+
>+    finishJSTest();
>+})();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 1ee35c5a1c96cd8c206b16ee465eae78d2e3bb15..2f569aacc55c45c7025ce216a70345729224bc6e 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -346,6 +346,12 @@ window.UIHelper = class UIHelper {
>         });
>     }
> 
>+    static zoomToScale(scale)
>+    {
>+        const uiScript = `uiController.zoomToScale(${scale}, () => uiController.uiScriptComplete())`;
>+        return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
>+    }
>+
>     static typeCharacter(characterString)
>     {
>         if (!this.isWebKit2() || !this.isIOS()) {

Actions: View | Formatted Diff | Diff

Attachments on bug 193005: 358019