Placeholder text is not repainted after caps lock indicator is hidden
authordbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 21:50:17 +0000 (21:50 +0000)
committerdbates@webkit.org <dbates@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 26 Nov 2018 21:50:17 +0000 (21:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=191968
<rdar://problem/46247234>

Reviewed by Zalan Bujtas.

Source/WebCore:

Fixes an issue where the placeholder text in a password field is not repainted when the
caps lock indicator is hidden.

The placeholder renderer is special. It is an excluded child renderer and does not take
part in normal flow layout. It is also created and destroyed as needed. The caps lock
indicator is also special in that it is implemented as a RenderImage and we do not know
its dimensions before it is loaded and the load happens asynchronously. As a result we
detect when the inner text size changes and mark the placeholder as dirty as a way to
keep the dimensions of the placeholder in sync with the dimensions of the inner text.

Test: fast/repaint/placeholder-after-caps-lock-hidden.html

* rendering/RenderTextControlSingleLine.cpp:
(WebCore::RenderTextControlSingleLine::layout): Mark the placeholder as needing layout
the size of the inner text changes.

LayoutTests:

Add a test to ensure to that the placeholder text is repainted when the caps lock indicator is hidden.

* TestExpectations: Skip the test on all platforms as we only support toggling Caps Lock in
WebKit2 on Mac at the moment.
* fast/repaint/placeholder-after-caps-lock-hidden.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@238519 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden-expected.txt [new file with mode: 0644]
LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden.html [new file with mode: 0644]
LayoutTests/platform/mac-wk2/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderTextControlSingleLine.cpp

index 7bc36eb..86756fb 100644 (file)
@@ -1,5 +1,20 @@
 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>
+
+        Reviewed by Zalan Bujtas.
+
+        Add a test to ensure to that the placeholder text is repainted when the caps lock indicator is hidden.
+
+        * TestExpectations: Skip the test on all platforms as we only support toggling Caps Lock in
+        WebKit2 on Mac at the moment.
+        * fast/repaint/placeholder-after-caps-lock-hidden.html: Added.
+        * platform/mac-wk2/TestExpectations: Mark the test as PASS so that we run it.
+
+2018-11-26  Daniel Bates  <dabates@apple.com>
+
         REGRESSION (r238078): Do not draw caps lock indicator when Strong Password button is shown
         https://bugs.webkit.org/show_bug.cgi?id=191969
         <rdar://problem/46247569>
index beeac91..da6019d 100644 (file)
@@ -401,6 +401,7 @@ fast/misc/valid-primary-screen-displayID.html [ Skip ]
 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 ]
 
 # This test currently only works for mac-wk2
 fast/events/inactive-window-no-mouse-event.html [ Skip ]
diff --git a/LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden-expected.txt b/LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden-expected.txt
new file mode 100644 (file)
index 0000000..170015e
--- /dev/null
@@ -0,0 +1,20 @@
+Tests that the placeholder text is repainted when the caps lock indicator is hidden.
+
+
+(repaint rects
+  (rect 31 50 22 22)
+  (rect 36 55 12 12)
+  (rect 8 50 33 23)
+  (rect 13 55 23 13)
+  (rect 8 50 45 23)
+  (rect 13 55 35 13)
+  (rect 31 50 22 23)
+  (rect 36 55 12 13)
+  (rect 8 50 45 23)
+  (rect 13 55 35 13)
+  (rect 8 50 33 23)
+  (rect 13 55 23 13)
+  (rect 8 50 45 23)
+  (rect 13 55 35 13)
+)
+
diff --git a/LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden.html b/LayoutTests/fast/repaint/placeholder-after-caps-lock-hidden.html
new file mode 100644 (file)
index 0000000..01bfd50
--- /dev/null
@@ -0,0 +1,58 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script src="../../resources/ui-helper.js"></script>
+<script>
+if (window.testRunner) {
+    testRunner.dumpAsText(true);
+    testRunner.waitUntilDone();
+}
+
+let step = 0;
+
+function handleKeyUp(event)
+{
+    switch (step++) {
+    case 0:
+        console.assert(event.key === "a");
+        UIHelper.keyDown("\b"); // Backspace
+        return;
+    case 1:
+        console.assert(event.key === "Backspace");
+        internals.startTrackingRepaints();
+        testRunner.toggleCapsLock();
+        return;
+    case 2: {
+        console.assert(event.key === "CapsLock");
+        document.getElementById("result").textContent = internals.repaintRectsAsText();
+        internals.stopTrackingRepaints();
+        testRunner.notifyDone();
+        return;
+    }
+    }
+}
+
+function runTest()
+{
+    if (!window.testRunner)
+        return;
+
+    let input = document.getElementById("input");
+    input.focus();
+
+    function handleCapsLockEnabled(event) {
+        console.assert(event.key === "CapsLock");
+        input.addEventListener("keyup", handleKeyUp, false);
+        UIHelper.keyDown("a");
+    }
+    input.addEventListener("keydown", handleCapsLockEnabled, { once: true });
+    testRunner.toggleCapsLock();
+}
+</script>
+</head>
+<body onload="runTest()">
+<p>Tests that the placeholder text is repainted when the caps lock indicator is hidden.</p>
+<input id="input" type="password" size="5" placeholder="Cupertino">
+<pre id="result"></pre>
+</body>
+</html>
index 88d4939..a5dbe07 100644 (file)
@@ -70,6 +70,7 @@ webkit.org/b/184569 storage/indexeddb/modern/transactions-stop-on-navigation.htm
 [ Mojave+ ] fast/events/detect-caps-lock.html [ Pass ]
 [ 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 ]
 
 fast/events/inactive-window-no-mouse-event.html [ Pass ]
 
index 0a33e4e..007321a 100644 (file)
@@ -1,3 +1,27 @@
+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>
+
+        Reviewed by Zalan Bujtas.
+
+        Fixes an issue where the placeholder text in a password field is not repainted when the
+        caps lock indicator is hidden.
+
+        The placeholder renderer is special. It is an excluded child renderer and does not take
+        part in normal flow layout. It is also created and destroyed as needed. The caps lock
+        indicator is also special in that it is implemented as a RenderImage and we do not know
+        its dimensions before it is loaded and the load happens asynchronously. As a result we
+        detect when the inner text size changes and mark the placeholder as dirty as a way to
+        keep the dimensions of the placeholder in sync with the dimensions of the inner text.
+
+        Test: fast/repaint/placeholder-after-caps-lock-hidden.html
+
+        * rendering/RenderTextControlSingleLine.cpp:
+        (WebCore::RenderTextControlSingleLine::layout): Mark the placeholder as needing layout
+        the size of the inner text changes.
+
 2018-11-26  Jeremy Jones  <jeremyj@apple.com>
 
         Picture-in-picture window size changes unnecesarily when URL changes.
index 23a3ab3..4e2375f 100644 (file)
@@ -110,6 +110,12 @@ 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.
+    LayoutSize oldInnerTextSize;
+    if (innerTextRenderer)
+        oldInnerTextSize = innerTextRenderer->size();
+
     RenderBlockFlow::layoutBlock(false);
 
     // Set the text block height
@@ -173,6 +179,10 @@ void RenderTextControlSingleLine::layout()
         placeholderBox->mutableStyle().setHeight(Length(innerTextSize.height() - placeholderBox->verticalBorderAndPaddingExtent(), Fixed));
         bool neededLayout = placeholderBox->needsLayout();
         bool placeholderBoxHadLayout = placeholderBox->everHadLayout();
+        if (innerTextSize != oldInnerTextSize) {
+            // The caps lock indicator was hidden. Layout the placeholder. Its layout does not affect its parent.
+            placeholderBox->setChildNeedsLayout(MarkOnlyThis);
+        }
         placeholderBox->layoutIfNeeded();
         LayoutPoint textOffset;
         if (innerTextRenderer)