Title: [191954] trunk
Revision
191954
Author
wenson_hs...@apple.com
Date
2015-11-03 09:15:38 -0800 (Tue, 03 Nov 2015)

Log Message

Tapping *below* some <input>s can focus them in Mobile Safari
https://bugs.webkit.org/show_bug.cgi?id=146244
<rdar://problem/21509310>

Reviewed by Darin Adler.

Source/WebCore:

Removes iOS-specific logic in positionForPointRespectingEditingBoundaries that was causing us to focus inputs by
tapping on the document element. We believe this logic, which causes VisiblePosition finding to recurse from a non-
editable element to an editable child, is not necessary to focus editable elements underneath non-editable elements,
since hit-testing will already have selected the contentEditable element prior to searching for a suitable
VisiblePosition. Further investigation shows that this logic was added to fix <rdar://problem/5545799>, in which the
first character in a Notes document could not be selected. However, I have not been able to reproduce this bug after
removing this logic.

As a result of this change, we can also enable a WK1 test, editing/selection/click-outside-editable-div.html, that
had also been marked as failing due to positionForPointRespectingEditingBoundaries recursing into a contentEditable
div.

Test: fast/events/ios/clicking-document-should-not-trigger-focus.html

* rendering/RenderBlock.cpp:
(WebCore::positionForPointRespectingEditingBoundaries): Deleted.

LayoutTests:

Enables a now-passing WK1 editing test (editing/selection/click-outside-editable-div.html) and also add a new
WK2 test that verifies an input node won't be focused by clicking anywhere in the document outside of the body.

* fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt: Added.
* fast/events/ios/clicking-document-should-not-trigger-focus.html: Added.
* platform/ios-simulator-wk1/TestExpectations: No longer disable editing/selection/click-outside-editable-div.html.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (191953 => 191954)


--- trunk/LayoutTests/ChangeLog	2015-11-03 17:13:24 UTC (rev 191953)
+++ trunk/LayoutTests/ChangeLog	2015-11-03 17:15:38 UTC (rev 191954)
@@ -1,3 +1,18 @@
+2015-11-02  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Tapping *below* some <input>s can focus them in Mobile Safari
+        https://bugs.webkit.org/show_bug.cgi?id=146244
+        <rdar://problem/21509310>
+
+        Reviewed by Darin Adler.
+
+        Enables a now-passing WK1 editing test (editing/selection/click-outside-editable-div.html) and also add a new
+        WK2 test that verifies an input node won't be focused by clicking anywhere in the document outside of the body.
+
+        * fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt: Added.
+        * fast/events/ios/clicking-document-should-not-trigger-focus.html: Added.
+        * platform/ios-simulator-wk1/TestExpectations: No longer disable editing/selection/click-outside-editable-div.html.
+
 2015-11-03  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Update to match text-orientation spec

Added: trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt (0 => 191954)


--- trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt	2015-11-03 17:15:38 UTC (rev 191954)
@@ -0,0 +1,4 @@
+This test is best run using WKTR. To test manually, click anywhere near the bottom of the page and verify that the input is not focused.
+
+The currently focused element is of type BODY
+

Added: trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus.html (0 => 191954)


--- trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus.html	                        (rev 0)
+++ trunk/LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus.html	2015-11-03 17:15:38 UTC (rev 191954)
@@ -0,0 +1,39 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+    <meta name="viewport" content="user-scalable  = no, width = device-width">
+    <head>
+        <script id="ui-script" type="text/plain">
+            (function() {
+                uiController.singleTapAtPoint(100, 300);
+            })();
+        </script>
+        <script>
+            if (window.testRunner) {
+                testRunner.dumpAsText();
+                testRunner.waitUntilDone();
+            }
+
+            function getUIScript()
+            {
+                return document.getElementById("ui-script").text;
+            }
+
+            function setup()
+            {
+                document.addEventListener("click", function() {
+                    document.body.appendChild(document.createTextNode("The currently focused element is of type " + document.activeElement.tagName));
+                    document.body.appendChild(document.createElement("br"));
+                    if (window.testRunner)
+                        testRunner.notifyDone();
+                });
+                if (window.testRunner && testRunner.runUIScript)
+                    testRunner.runUIScript(getUIScript(), function() { });
+            }
+        </script>
+    </head>
+    <body _onload_="setup()" style="height: 50px;">
+        <p>This test is best run using WKTR. To test manually, click anywhere near the bottom of the page and verify that the input is not focused.</p>
+        <input style="display: block;">
+    </body>
+</html>

Modified: trunk/LayoutTests/platform/ios-simulator-wk1/TestExpectations (191953 => 191954)


--- trunk/LayoutTests/platform/ios-simulator-wk1/TestExpectations	2015-11-03 17:13:24 UTC (rev 191953)
+++ trunk/LayoutTests/platform/ios-simulator-wk1/TestExpectations	2015-11-03 17:15:38 UTC (rev 191954)
@@ -298,7 +298,6 @@
 editing/selection/caret-in-empty-inline-2.html [ Failure ]
 editing/selection/caret-mode-paragraph-keys-navigation.html [ Failure ]
 editing/selection/click-in-focusable-link-should-not-clear-selection.html [ Failure ]
-editing/selection/click-outside-editable-div.html [ Failure ]
 editing/selection/collapse-selection-in-bidi.html [ Failure ]
 editing/selection/context-menu-on-text.html [ Failure ]
 editing/selection/context-menu-text-selection.html [ Failure ]

Modified: trunk/Source/WebCore/ChangeLog (191953 => 191954)


--- trunk/Source/WebCore/ChangeLog	2015-11-03 17:13:24 UTC (rev 191953)
+++ trunk/Source/WebCore/ChangeLog	2015-11-03 17:15:38 UTC (rev 191954)
@@ -1,3 +1,28 @@
+2015-11-02  Wenson Hsieh  <wenson_hs...@apple.com>
+
+        Tapping *below* some <input>s can focus them in Mobile Safari
+        https://bugs.webkit.org/show_bug.cgi?id=146244
+        <rdar://problem/21509310>
+
+        Reviewed by Darin Adler.
+
+        Removes iOS-specific logic in positionForPointRespectingEditingBoundaries that was causing us to focus inputs by
+        tapping on the document element. We believe this logic, which causes VisiblePosition finding to recurse from a non-
+        editable element to an editable child, is not necessary to focus editable elements underneath non-editable elements,
+        since hit-testing will already have selected the contentEditable element prior to searching for a suitable
+        VisiblePosition. Further investigation shows that this logic was added to fix <rdar://problem/5545799>, in which the
+        first character in a Notes document could not be selected. However, I have not been able to reproduce this bug after
+        removing this logic.
+
+        As a result of this change, we can also enable a WK1 test, editing/selection/click-outside-editable-div.html, that
+        had also been marked as failing due to positionForPointRespectingEditingBoundaries recursing into a contentEditable
+        div.
+
+        Test: fast/events/ios/clicking-document-should-not-trigger-focus.html
+
+        * rendering/RenderBlock.cpp:
+        (WebCore::positionForPointRespectingEditingBoundaries): Deleted.
+
 2015-11-03  Myles C. Maxfield  <mmaxfi...@apple.com>
 
         Update to match text-orientation spec

Modified: trunk/Source/WebCore/rendering/RenderBlock.cpp (191953 => 191954)


--- trunk/Source/WebCore/rendering/RenderBlock.cpp	2015-11-03 17:13:24 UTC (rev 191953)
+++ trunk/Source/WebCore/rendering/RenderBlock.cpp	2015-11-03 17:15:38 UTC (rev 191954)
@@ -2556,15 +2556,6 @@
     // If we can't find an ancestor to check editability on, or editability is unchanged, we recur like normal
     if (isEditingBoundary(ancestor, child))
         return child.positionForPoint(pointInChildCoordinates, nullptr);
-    
-#if PLATFORM(IOS)
-    // On iOS we want to constrain VisiblePositions to the editable region closest to the input position, so
-    // we will allow descent from non-editable to editable content.
-    // FIXME: This constraining must be done at a higher level once we implement contentEditable. For now, if something
-    // is editable, the whole document will be.
-    if (childElement->isContentEditable() && !ancestor->element()->isContentEditable())
-        return child.positionForPoint(pointInChildCoordinates, nullptr);
-#endif
 
     // Otherwise return before or after the child, depending on if the click was to the logical left or logical right of the child
     LayoutUnit childMiddle = parent.logicalWidthForChild(child) / 2;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to