Title: [238522] trunk
Revision
238522
Author
[email protected]
Date
2018-11-26 14:15:16 -0800 (Mon, 26 Nov 2018)

Log Message

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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (238521 => 238522)


--- trunk/LayoutTests/ChangeLog	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/LayoutTests/ChangeLog	2018-11-26 22:15:16 UTC (rev 238522)
@@ -1,5 +1,22 @@
 2018-11-26  Daniel Bates  <[email protected]>
 
+        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  <[email protected]>
+
         Placeholder text is not repainted after caps lock indicator is hidden
         https://bugs.webkit.org/show_bug.cgi?id=191968
         <rdar://problem/46247234>

Modified: trunk/LayoutTests/TestExpectations (238521 => 238522)


--- trunk/LayoutTests/TestExpectations	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/LayoutTests/TestExpectations	2018-11-26 22:15:16 UTC (rev 238522)
@@ -402,6 +402,7 @@
 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 ]

Added: trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt (0 => 238522)


--- trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled-expected.txt	2018-11-26 22:15:16 UTC (rev 238522)
@@ -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
+

Added: trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html (0 => 238522)


--- trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html	                        (rev 0)
+++ trunk/LayoutTests/fast/forms/password-scrolled-after-caps-lock-toggled.html	2018-11-26 22:15:16 UTC (rev 238522)
@@ -0,0 +1,174 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src=""
+<Script src=""
+<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>

Modified: trunk/LayoutTests/platform/mac-wk2/TestExpectations (238521 => 238522)


--- trunk/LayoutTests/platform/mac-wk2/TestExpectations	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/LayoutTests/platform/mac-wk2/TestExpectations	2018-11-26 22:15:16 UTC (rev 238522)
@@ -71,6 +71,7 @@
 [ 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 ]
 

Modified: trunk/Source/WebCore/ChangeLog (238521 => 238522)


--- trunk/Source/WebCore/ChangeLog	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/Source/WebCore/ChangeLog	2018-11-26 22:15:16 UTC (rev 238522)
@@ -1,5 +1,38 @@
 2018-11-26  Daniel Bates  <[email protected]>
 
+        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  <[email protected]>
+
         Placeholder text is not repainted after caps lock indicator is hidden
         https://bugs.webkit.org/show_bug.cgi?id=191968
         <rdar://problem/46247234>

Modified: trunk/Source/WebCore/editing/FrameSelection.cpp (238521 => 238522)


--- trunk/Source/WebCore/editing/FrameSelection.cpp	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/Source/WebCore/editing/FrameSelection.cpp	2018-11-26 22:15:16 UTC (rev 238522)
@@ -403,9 +403,11 @@
 #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();

Modified: trunk/Source/WebCore/editing/FrameSelection.h (238521 => 238522)


--- trunk/Source/WebCore/editing/FrameSelection.h	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/Source/WebCore/editing/FrameSelection.h	2018-11-26 22:15:16 UTC (rev 238522)
@@ -159,8 +159,10 @@
 
     void updateAppearanceAfterLayout();
     void scheduleAppearanceUpdateAfterStyleChange();
-    void setNeedsSelectionUpdate();
 
+    enum class RevealSelectionAfterUpdate : bool { NotForced, Forced };
+    void setNeedsSelectionUpdate(RevealSelectionAfterUpdate = RevealSelectionAfterUpdate::NotForced);
+
     bool contains(const LayoutPoint&) const;
 
     WEBCORE_EXPORT bool modify(EAlteration, SelectionDirection, TextGranularity, EUserTriggered = NotUserTriggered);

Modified: trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp (238521 => 238522)


--- trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp	2018-11-26 21:59:16 UTC (rev 238521)
+++ trunk/Source/WebCore/rendering/RenderTextControlSingleLine.cpp	2018-11-26 22:15:16 UTC (rev 238522)
@@ -110,8 +110,9 @@
     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 @@
     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 @@
         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 @@
     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)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to