Changeset 240452 in webkit

Timestamp:

Jan 24, 2019 3:30:37 PM (5 years ago)

Author:

Wenson Hsieh

Message:

[iOS] Unable to make a selection in jsfiddle.net using arrow keys when requesting desktop site
​https://bugs.webkit.org/show_bug.cgi?id=193758
<rdar://problem/43614978>

Reviewed by Tim Horton.

Source/WebCore:

CodeMirror's script adds a repeating timer that periodically normalizes the logical selection in the editor
(this is distinct from the actual DOM selection, which is inside a hidden textarea element). This script defines
a helper method to select all the text inside of a text form control, called selectInput, which normally
invokes node.select(). However, in the case of iOS, the script works around broken select() behavior by
setting selectionStart and selectionEnd to encompass all the content in the form control. When requesting
the desktop version of the site, CodeMirror no longer attempts to apply its iOS workaround.

This iOS-specific behavior was introduced to fix <rdar://problem/4901923>. However, the original bug no longer
reproduces even without this quirk. To fix CodeMirror, we make two adjustments:

1. Roll out this ancient demo hack, in favor of standardized behavior.
2. Note that select() is also used when focusing an input. However, when focusing an input element on iOS, we

want to match the platform (i.e. UITextField behavior) and move focus to the end of the text field. To
achieve this, we introduce a new helper on HTMLInputElement that is called when setting the default
selection of a text input after focus; on iOS, this helper method moves the selection to the end of the
input, but everywhere else, it selects all the text in the input element.

This causes 6 existing layout tests to begin passing on iOS.

• html/HTMLInputElement.cpp:

(WebCore::HTMLInputElement::updateFocusAppearance):
(WebCore::HTMLInputElement::setDefaultSelectionAfterFocus):

• html/HTMLInputElement.h:
• html/HTMLTextFormControlElement.cpp:

(WebCore::HTMLTextFormControlElement::select):

LayoutTests:

Mark some existing layout tests as passing on iOS. Additionally, remove failing expectations for another
existing layout test on iOS.

• platform/ios/TestExpectations:
• platform/ios/editing/text-iterator/hidden-textarea-selection-quirk-expected.txt: Removed.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios/TestExpectations (modified) (4 diffs)
• LayoutTests/platform/ios/editing/text-iterator (deleted)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/html/HTMLInputElement.cpp (modified) (1 diff)
• Source/WebCore/html/HTMLInputElement.h (modified) (1 diff)
• Source/WebCore/html/HTMLTextFormControlElement.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-01-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Unable to make a selection in jsfiddle.net using arrow keys when requesting desktop site
+        https://bugs.webkit.org/show_bug.cgi?id=193758
+        <rdar://problem/43614978>
+
+        Reviewed by Tim Horton.
+
+        Mark some existing layout tests as passing on iOS. Additionally, remove failing expectations for another
+        existing layout test on iOS.
+
+        * platform/ios/TestExpectations:
+        * platform/ios/editing/text-iterator/hidden-textarea-selection-quirk-expected.txt: Removed.
+
 2019-01-24  John Wilander  <wilander@apple.com>
 

trunk/LayoutTests/platform/ios/TestExpectations

@@ -1683 +1683 @@
 fast/forms/input-live-pseudo-selectors.html [ Failure ]
 fast/forms/input-no-renderer.html [ Failure ]
-fast/forms/input-select-webkit-user-select-none.html [ Failure ]
 fast/forms/input-set-composition-scroll.html [ Failure ]
 fast/forms/input-textarea-padding-match.html [ ImageOnlyFailure ]
@@ -1708 +1707 @@
 fast/forms/select-overflow-scroll.html [ Failure ]
 fast/forms/select/option-selecting.html [ Failure ]
-fast/forms/shadow-tree-exposure.html [ Failure ]
 fast/forms/textarea-input-event.html [ Failure ]
 fast/forms/textarea-live-pseudo-selectors.html [ Failure ]
 fast/forms/textarea-metrics.html [ Failure ]
 fast/forms/textarea-placeholder-wrapping.html [ ImageOnlyFailure ]
-fast/forms/textarea-set-defaultvalue-after-value.html [ Failure ]
 fast/forms/textfield-overflow-by-value-update.html [ Failure ]
 fast/frames/calculate-fixed.html [ Failure ]
@@ -2118 +2115 @@
 editing/pasteboard/copy-inside-h1-preserves-h1.html [ Failure ]
 editing/pasteboard/copy-text-with-backgroundcolor.html [ Failure ]
-editing/pasteboard/copy-two-pasteboard-types-both-work.html [ Failure ]
 webkit.org/b/177961 editing/pasteboard/data-transfer-items.html [ Skip ]
 editing/pasteboard/dataTransfer-setData-getData.html [ Failure ]
@@ -2138 +2134 @@
 editing/pasteboard/paste-global-selection.html [ Failure ]
 editing/pasteboard/paste-list-004.html [ Failure ]
-editing/pasteboard/paste-placeholder-input.html [ Failure ]
 editing/pasteboard/paste-plaintext-user-select-none.html [ Failure ]
 editing/pasteboard/paste-sanitize-crash-1.html [ Failure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-01-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        [iOS] Unable to make a selection in jsfiddle.net using arrow keys when requesting desktop site
+        https://bugs.webkit.org/show_bug.cgi?id=193758
+        <rdar://problem/43614978>
+
+        Reviewed by Tim Horton.
+
+        CodeMirror's script adds a repeating timer that periodically normalizes the logical selection in the editor
+        (this is distinct from the actual DOM selection, which is inside a hidden textarea element). This script defines
+        a helper method to select all the text inside of a text form control, called `selectInput`, which normally
+        invokes `node.select()`. However, in the case of iOS, the script works around broken `select()` behavior by
+        setting `selectionStart` and `selectionEnd` to encompass all the content in the form control. When requesting
+        the desktop version of the site, CodeMirror no longer attempts to apply its iOS workaround.
+
+        This iOS-specific behavior was introduced to fix <rdar://problem/4901923>. However, the original bug no longer
+        reproduces even without this quirk. To fix CodeMirror, we make two adjustments:
+
+        1.  Roll out this ancient demo hack, in favor of standardized behavior.
+        2.  Note that `select()` is also used when focusing an input. However, when focusing an input element on iOS, we
+            want to match the platform (i.e. UITextField behavior) and move focus to the end of the text field. To
+            achieve this, we introduce a new helper on HTMLInputElement that is called when setting the default
+            selection of a text input after focus; on iOS, this helper method moves the selection to the end of the
+            input, but everywhere else, it selects all the text in the input element.
+
+        This causes 6 existing layout tests to begin passing on iOS.
+
+        * html/HTMLInputElement.cpp:
+        (WebCore::HTMLInputElement::updateFocusAppearance):
+        (WebCore::HTMLInputElement::setDefaultSelectionAfterFocus):
+        * html/HTMLInputElement.h:
+        * html/HTMLTextFormControlElement.cpp:
+        (WebCore::HTMLTextFormControlElement::select):
+
 2019-01-24  Jer Noble  <jer.noble@apple.com>
 

trunk/Source/WebCore/html/HTMLInputElement.cpp

@@ -460 +460 @@
     if (isTextField()) {
         if (restorationMode == SelectionRestorationMode::SetDefault || !hasCachedSelection())
-            select(revealMode, Element::defaultFocusTextStateChangeIntent());
+            setDefaultSelectionAfterFocus(revealMode);
         else
             restoreCachedSelection(revealMode);
     } else
         HTMLTextFormControlElement::updateFocusAppearance(restorationMode, revealMode);
+}
+
+void HTMLInputElement::setDefaultSelectionAfterFocus(SelectionRevealMode revealMode)
+{
+    ASSERT(isTextField());
+#if PLATFORM(IOS_FAMILY)
+    // We don't want to select all the text on iOS when focusing a field. Instead, match platform behavior by going to the end of the line.
+    int start = std::numeric_limits<int>::max();
+    auto direction = SelectionHasForwardDirection;
+#else
+    int start = 0;
+    auto direction = SelectionHasNoDirection;
+#endif
+    setSelectionRange(start, std::numeric_limits<int>::max(), direction, revealMode, Element::defaultFocusTextStateChangeIntent());
 }
 

trunk/Source/WebCore/html/HTMLInputElement.h

@@ -455 +455 @@
     void removeFromRadioButtonGroup();
 
+    void setDefaultSelectionAfterFocus(SelectionRevealMode);
+
     AtomicString m_name;
     String m_valueIfDirty;

trunk/Source/WebCore/html/HTMLTextFormControlElement.cpp

@@ -190 +190 @@
 void HTMLTextFormControlElement::select(SelectionRevealMode revealMode, const AXTextStateChangeIntent& intent)
 {
-    // FIXME: We should abstract the selection behavior into an EditingBehavior function instead
-    // of hardcoding the behavior using a macro define.
-#if PLATFORM(IOS_FAMILY)
-    // We don't want to select all the text on iOS. Instead use the standard textfield behavior of going to the end of the line.
-    setSelectionRange(std::numeric_limits<int>::max(), std::numeric_limits<int>::max(), SelectionHasForwardDirection, revealMode, intent);
-#else
     setSelectionRange(0, std::numeric_limits<int>::max(), SelectionHasNoDirection, revealMode, intent);
-#endif
 }