Changeset 191954 in webkit

Timestamp:

Nov 3, 2015 9:15:38 AM (8 years ago)

Author:

Wenson Hsieh

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus-expected.txt (added)
• LayoutTests/fast/events/ios/clicking-document-should-not-trigger-focus.html (added)
• LayoutTests/platform/ios-simulator-wk1/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderBlock.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2015-11-02  Wenson Hsieh  <wenson_hsieh@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  <mmaxfield@apple.com>
 

trunk/LayoutTests/platform/ios-simulator-wk1/TestExpectations

@@ -299 +299 @@
 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 ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2015-11-02  Wenson Hsieh  <wenson_hsieh@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  <mmaxfield@apple.com>
 

trunk/Source/WebCore/rendering/RenderBlock.cpp

@@ -2557 +2557 @@
     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