Caret disappears at end of password field when caps lock indicator is shown; password...
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 22:15:16 +0000 (22:15 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 22:15:16 +0000 (22:15 +0000)
not scrolled when caps lock indicator is shown
https://bugs.webkit.org/show_bug.cgi?id=191164
<rdar://problem/45738179>

Reviewed by Dean Jackson.

Source/WebCore:

Fixes an issue where the caret may be occluded by- or paint on top of- the caps lock indicator on
Mac and iOS, respectively.

If there has not been a previous selection in a focused password field, including a caret
selection made by pressing the arrow keys, then we never scroll the password field to reveal
the current selection when the caps lock indicator is made visible. When the caps lock indicator
is made visible or hidden the size of the inner text renderer changes as it shrinks or expands
to make space for the caps lock indicator or to fill the void of the now hidden caps lock indicator,
respectively. We should detect such size changes and schedule an update and reveal of the current
selection after layout.

Test: fast/forms/password-scrolled-after-caps-lock-toggled.html

* editing/FrameSelection.cpp:
(WebCore::FrameSelection::setNeedsSelectionUpdate): Modified to take an enum to override the current
selection reveal mode for the next update.
* editing/FrameSelection.h:
* rendering/RenderTextControlSingleLine.cpp:
(WebCore::RenderTextControlSingleLine::layout): Schedule post-layout a selection update that
reveals the current selection. We pass FrameSelection::RevealSelectionAfterUpdate::Forced to ensure
that the scheduled selection update scrolls to the reveal the current selection regardless of selection
reveal mode. This is necessary because typing into a password field does not change the current
selection reveal mode.

LayoutTests:

Add a test to ensure that we scroll the password field when caps lock is toggled.

* TestExpectations: Skip the test on all platforms as we only support toggling Caps Lock in
WebKit2 on Mac at the moment.
* fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt: Added.
* fast/forms/password-scrolled-after-caps-lock-toggled.html: Added.
* platform/mac-wk2/TestExpectations: Mark the test as PASS so that we run it.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@238522 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt [new file with mode: 0644]
LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/editing/FrameSelection.cpp
Source/WebCore/editing/FrameSelection.h
Source/WebCore/rendering/RenderTextControlSingleLine.cpp

index 6ced031..9c5029d 100644 (file)
@@ -1,5 +1,22 @@
 2018-11-26  Daniel Bates  <dabates@apple.com>
 
+        Caret disappears at end of password field when caps lock indicator is shown; password field
+        not scrolled when caps lock indicator is shown
+        https://bugs.webkit.org/show_bug.cgi?id=191164
+        <rdar://problem/45738179>
+
+        Reviewed by Dean Jackson.
+
+        Add a test to ensure that we scroll the password field when caps lock is toggled.
+
+        * TestExpectations: Skip the test on all platforms as we only support toggling Caps Lock in
+        WebKit2 on Mac at the moment.
+        * fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt: Added.
+        * fast/forms/password-scrolled-after-caps-lock-toggled.html: Added.
+        * platform/mac-wk2/TestExpectations: Mark the test as PASS so that we run it.
+
+2018-11-26  Daniel Bates  <dabates@apple.com>
+
         Placeholder text is not repainted after caps lock indicator is hidden
         https://bugs.webkit.org/show_bug.cgi?id=191968
         <rdar://problem/46247234>
index da6019d..a6ddbe1 100644 (file)
@@ -402,6 +402,7 @@ fast/events/detect-caps-lock.html [ Skip ]
 fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after-hiding-auto-fill-strong-password-button.html [ Skip ]
 fast/forms/auto-fill-button/caps-lock-indicator-should-not-be-visible-when-auto-fill-strong-password-button-is-visible.html [ Skip ]
 fast/repaint/placeholder-after-caps-lock-hidden.html [ Skip ]
+fast/forms/password-scrolled-after-caps-lock-toggled.html [ Skip ]
 
 # This test currently only works for mac-wk2
 fast/events/inactive-window-no-mouse-event.html [ Skip ]
diff --git a/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt b/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt
new file mode 100644 (file)
index 0000000..ecbf515
--- /dev/null
@@ -0,0 +1,44 @@
+Tests that the password field is scrolled when the caps lock indicator is toggled.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+Case 1: Empty field:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock enabled:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock disabled:
+PASS document.getElementById('input').scrollLeft is 0
+
+Case 2: After typing into field:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock enabled:
+PASS document.getElementById('input').scrollLeft is non-zero.
+
+After caps lock disabled:
+PASS document.getElementById('input').scrollLeft is 0
+
+Case 3: After selecting the first 2 characters:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock enabled:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock disabled:
+PASS document.getElementById('input').scrollLeft is 0
+
+Case 4: After selecting the last 2 characters:
+PASS document.getElementById('input').scrollLeft is 0
+
+After caps lock enabled:
+PASS document.getElementById('input').scrollLeft is non-zero.
+
+After caps lock disabled:
+PASS document.getElementById('input').scrollLeft is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html b/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html
new file mode 100644 (file)
index 0000000..33fd6b2
--- /dev/null
@@ -0,0 +1,174 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/js-test.js"></script>
+<Script src="../../resources/ui-helper.js"></Script>
+<script>
+window.jsTestIsAsync = true;
+
+let input;
+let numberOfCharactersToOverflowFieldWhenCapsLockShown;
+
+let tests = [
+    // Test for caret selection
+    // Empty field
+    testFocusedEmptyFieldIsNotScrolled,
+    testFieldDidNotScrollAfterCapsLockEnabled,
+    testFieldDidNotScrollAfterCapsLockDisabled,
+
+    // Non-empty field
+    testFieldDidNotScrollAfterTyping,
+    testFieldDidScrollAfterCapsLockEnabled,
+    testFieldDidScrollAfterCapsLockDisabled,
+
+    // Test for range selection
+    // Select the first few characters
+    testFieldDidNotScrollAfterSelectingPrefix,
+    testFieldDidNotScrollAfterCapsLockEnabled,
+    testFieldDidNotScrollAfterCapsLockDisabled,
+
+    // Select the last few characters
+    testFieldDidNotScrollAfterSelectingSuffix,
+    testFieldDidScrollAfterCapsLockEnabled,
+    testFieldDidScrollAfterCapsLockDisabled,
+];
+
+let currentTest = 0;
+
+function runNextTest()
+{
+    if (currentTest >= tests.length) {
+        done();
+        return;
+    }
+    tests[currentTest++]();
+}
+
+function runTest()
+{
+    runNextTest();
+}
+
+function testFocusedEmptyFieldIsNotScrolled()
+{
+    debug("Case 1: Empty field:");
+    input.focus();
+    shouldBeZero("document.getElementById('input').scrollLeft");
+    runNextTest();
+}
+
+function testFieldDidNotScrollAfterTyping()
+{
+    function checkFieldFilled(event) {
+        console.assert(event.target === input);
+        if (input.value.length < numberOfCharactersToOverflowFieldWhenCapsLockShown)
+            return;
+        input.oninput = null;
+        shouldBeZero("document.getElementById('input').scrollLeft");
+        runNextTest();
+    }
+    input.oninput = checkFieldFilled;
+
+    debug("<br>Case 2: After typing into field:");
+    if (window.testRunner) {
+        for (let i = 0; i < numberOfCharactersToOverflowFieldWhenCapsLockShown; ++i)
+            UIHelper.keyDown("a");
+    }
+}
+
+function _toggleCapsLockWithCallbackAndRunNextTest(expectCapsLockEnabled, callback)
+{
+    if (window.internals)
+        console.assert(internals.capsLockIsOn() != expectCapsLockEnabled);
+    let newCapsLockStateDisplayName;
+    let eventToListenFor;
+    if (expectCapsLockEnabled) {
+        newCapsLockStateDisplayName = "enabled";
+        eventToListenFor = "keydown";
+    } else {
+        newCapsLockStateDisplayName = "disabled";
+        eventToListenFor = "keyup";
+    }
+
+    let oldValue = document.getElementById('input').scrollLeft;
+    function handleCapsLockChange(event) {
+        console.assert(event.key === "CapsLock");
+        callback(oldValue, document.getElementById('input').scrollLeft);
+        runNextTest();
+    }
+    input.addEventListener(eventToListenFor, handleCapsLockChange, { once: true });
+    debug(`<br>After caps lock ${newCapsLockStateDisplayName}:`);
+    if (window.testRunner)
+        UIHelper.toggleCapsLock();
+}
+
+function testFieldDidScrollAfterCapsLockEnabled()
+{
+    _toggleCapsLockWithCallbackAndRunNextTest(true, (oldValue, newValue) => {
+        console.assert(oldValue != newValue);
+        shouldBeNonZero("document.getElementById('input').scrollLeft");
+    });
+}
+
+function testFieldDidScrollAfterCapsLockDisabled()
+{
+    _toggleCapsLockWithCallbackAndRunNextTest(false, (oldValue, newValue) => {
+        console.assert(oldValue != newValue);
+        shouldBeZero("document.getElementById('input').scrollLeft");
+    });
+}
+
+function testFieldDidNotScrollAfterCapsLockEnabled()
+{
+    _toggleCapsLockWithCallbackAndRunNextTest(true, (oldValue, newValue) => {
+        console.assert(oldValue == newValue);
+        shouldBeZero("document.getElementById('input').scrollLeft");
+    });
+}
+
+function testFieldDidNotScrollAfterCapsLockDisabled()
+{
+    _toggleCapsLockWithCallbackAndRunNextTest(false, (oldValue, newValue) => {
+        console.assert(oldValue == newValue);
+        shouldBeZero("document.getElementById('input').scrollLeft");
+    });
+}
+
+function testFieldDidNotScrollAfterSelectingPrefix()
+{
+    let prefixLength = Math.floor(numberOfCharactersToOverflowFieldWhenCapsLockShown / 2);
+    debug(`<br>Case 3: After selecting the first ${prefixLength} characters:`);
+    input.setSelectionRange(0, prefixLength);
+    shouldBeZero("document.getElementById('input').scrollLeft");
+    runNextTest();
+}
+
+function testFieldDidNotScrollAfterSelectingSuffix()
+{
+    let suffixLength = Math.floor(numberOfCharactersToOverflowFieldWhenCapsLockShown / 2);
+    debug(`<br>Case 4: After selecting the last ${suffixLength} characters:`);
+    input.setSelectionRange(input.value.length - suffixLength, input.value.length);
+    shouldBeZero("document.getElementById('input').scrollLeft");
+    runNextTest();
+}
+
+function done()
+{
+    if (window.testRunner)
+        document.body.removeChild(document.getElementById("input"));
+    finishJSTest();
+}
+</script>
+</head>
+<body onload="runTest()">
+<input type="password" id="input" size="5">
+<script>
+input = document.getElementById("input");
+console.assert(input.hasAttribute("size"));
+numberOfCharactersToOverflowFieldWhenCapsLockShown = input.size;
+console.assert(numberOfCharactersToOverflowFieldWhenCapsLockShown >= 3);
+
+description("Tests that the password field is scrolled when the caps lock indicator is toggled.");
+</script>
+</body>
+</html>
index a5dbe07..76c6e72 100644 (file)
@@ -71,6 +71,7 @@ webkit.org/b/184569 storage/indexeddb/modern/transactions-stop-on-navigation.htm
 [ Mojave+ ] fast/forms/auto-fill-button/caps-lock-indicator-should-be-visible-when-after-hiding-auto-fill-strong-password-button.html [ Pass ]
 [ Mojave+ ] fast/forms/auto-fill-button/caps-lock-indicator-should-not-be-visible-when-auto-fill-strong-password-button-is-visible.html [ Pass ]
 [ Mojave+ ] fast/repaint/placeholder-after-caps-lock-hidden.html [ Pass ]
+[ Mojave+ ] fast/forms/password-scrolled-after-caps-lock-toggled.html [ Pass ]
 
 fast/events/inactive-window-no-mouse-event.html [ Pass ]
 
index 007321a..31bb81f 100644 (file)
@@ -1,5 +1,38 @@
 2018-11-26  Daniel Bates  <dabates@apple.com>
 
+        Caret disappears at end of password field when caps lock indicator is shown; password field
+        not scrolled when caps lock indicator is shown
+        https://bugs.webkit.org/show_bug.cgi?id=191164
+        <rdar://problem/45738179>
+
+        Reviewed by Dean Jackson.
+
+        Fixes an issue where the caret may be occluded by- or paint on top of- the caps lock indicator on
+        Mac and iOS, respectively.
+
+        If there has not been a previous selection in a focused password field, including a caret
+        selection made by pressing the arrow keys, then we never scroll the password field to reveal
+        the current selection when the caps lock indicator is made visible. When the caps lock indicator
+        is made visible or hidden the size of the inner text renderer changes as it shrinks or expands
+        to make space for the caps lock indicator or to fill the void of the now hidden caps lock indicator,
+        respectively. We should detect such size changes and schedule an update and reveal of the current
+        selection after layout.
+
+        Test: fast/forms/password-scrolled-after-caps-lock-toggled.html
+
+        * editing/FrameSelection.cpp:
+        (WebCore::FrameSelection::setNeedsSelectionUpdate): Modified to take an enum to override the current
+        selection reveal mode for the next update.
+        * editing/FrameSelection.h:
+        * rendering/RenderTextControlSingleLine.cpp:
+        (WebCore::RenderTextControlSingleLine::layout): Schedule post-layout a selection update that
+        reveals the current selection. We pass FrameSelection::RevealSelectionAfterUpdate::Forced to ensure
+        that the scheduled selection update scrolls to the reveal the current selection regardless of selection
+        reveal mode. This is necessary because typing into a password field does not change the current
+        selection reveal mode.
+
+2018-11-26  Daniel Bates  <dabates@apple.com>
+
         Placeholder text is not repainted after caps lock indicator is hidden
         https://bugs.webkit.org/show_bug.cgi?id=191968
         <rdar://problem/46247234>
index 424e32e..53d658c 100644 (file)
@@ -403,9 +403,11 @@ static void updateSelectionByUpdatingLayoutOrStyle(Frame& frame)
 #endif
 }
 
-void FrameSelection::setNeedsSelectionUpdate()
+void FrameSelection::setNeedsSelectionUpdate(RevealSelectionAfterUpdate revealMode)
 {
     m_selectionRevealIntent = AXTextStateChangeIntent();
+    if (revealMode == RevealSelectionAfterUpdate::Forced)
+        m_selectionRevealMode = SelectionRevealMode::Reveal;
     m_pendingSelectionUpdate = true;
     if (RenderView* view = m_frame->contentRenderer())
         view->selection().clear();
index 6d752df..23154ad 100644 (file)
@@ -159,7 +159,9 @@ public:
 
     void updateAppearanceAfterLayout();
     void scheduleAppearanceUpdateAfterStyleChange();
-    void setNeedsSelectionUpdate();
+
+    enum class RevealSelectionAfterUpdate : bool { NotForced, Forced };
+    void setNeedsSelectionUpdate(RevealSelectionAfterUpdate = RevealSelectionAfterUpdate::NotForced);
 
     bool contains(const LayoutPoint&) const;
 
index 4e2375f..298fac2 100644 (file)
@@ -110,8 +110,9 @@ void RenderTextControlSingleLine::layout()
     resetOverriddenHeight(innerBlockRenderer, this);
     resetOverriddenHeight(containerRenderer, this);
 
-    // Save the old size of the inner text (if we have one) as we will need to layout the placeholder if
-    // it changes to keep the size of the placeholder proportional to the size of the inner text.
+    // Save the old size of the inner text (if we have one) as we will need to layout the placeholder
+    // and update selection if it changes. One way the size may change is if text decorations are
+    // toggled. For example, hiding and showing the caps lock indicator will cause a size change.
     LayoutSize oldInnerTextSize;
     if (innerTextRenderer)
         oldInnerTextSize = innerTextRenderer->size();
@@ -170,6 +171,8 @@ void RenderTextControlSingleLine::layout()
     else if (container && containerRenderer && containerRenderer->height() != contentLogicalHeight())
         centerRenderer(*containerRenderer);
 
+    bool innerTextSizeChanged = innerTextRenderer && innerTextRenderer->size() != oldInnerTextSize;
+
     HTMLElement* placeholderElement = inputElement().placeholderElement();
     if (RenderBox* placeholderBox = placeholderElement ? placeholderElement->renderBox() : 0) {
         LayoutSize innerTextSize;
@@ -179,7 +182,7 @@ void RenderTextControlSingleLine::layout()
         placeholderBox->mutableStyle().setHeight(Length(innerTextSize.height() - placeholderBox->verticalBorderAndPaddingExtent(), Fixed));
         bool neededLayout = placeholderBox->needsLayout();
         bool placeholderBoxHadLayout = placeholderBox->everHadLayout();
-        if (innerTextSize != oldInnerTextSize) {
+        if (innerTextSizeChanged) {
             // The caps lock indicator was hidden. Layout the placeholder. Its layout does not affect its parent.
             placeholderBox->setChildNeedsLayout(MarkOnlyThis);
         }
@@ -209,6 +212,12 @@ void RenderTextControlSingleLine::layout()
     if (inputElement().isSearchField())
         RenderThemeIOS::adjustRoundBorderRadius(mutableStyle(), *this);
 #endif
+    if (innerTextSizeChanged) {
+        // The caps lock indicator was hidden or shown. If it is now visible then it may be occluding
+        // the current selection (say, the caret was after the last character in the text field).
+        // Schedule an update and reveal of the current selection.
+        frame().selection().setNeedsSelectionUpdate(FrameSelection::RevealSelectionAfterUpdate::Forced);
+    }
 }
 
 bool RenderTextControlSingleLine::nodeAtPoint(const HitTestRequest& request, HitTestResult& result, const HitTestLocation& locationInContainer, const LayoutPoint& accumulatedOffset, HitTestAction hitTestAction)