Bug 1637476 - Build the ::placeholder's DisplayLists first so that it doesn't obscure the caret. r=emilio
☠☠ backed out by 3ac5aa91c2ea ☠ ☠
authorMats Palmgren <mats@mozilla.com>
Wed, 20 May 2020 16:22:49 +0000
changeset 531279 55bb4c4449cd663e0de66f152845054f9cf6af37
parent 531278 64f2ad721dbf0bcbbdad1109aa147b83f888b814
child 531280 3ac5aa91c2ea39e70af36ff4bedf46726515522b
push id37436
push userncsoregi@mozilla.com
push dateWed, 20 May 2020 21:30:50 +0000
treeherdermozilla-central@6c10970490f3 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersemilio
bugs1637476
milestone78.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1637476 - Build the ::placeholder's DisplayLists first so that it doesn't obscure the caret. r=emilio Differential Revision: https://phabricator.services.mozilla.com/D75252
layout/base/tests/bug1637476-1-ref.html
layout/base/tests/bug1637476-1.html
layout/base/tests/bug1637476-2-ref.html
layout/base/tests/bug1637476-2.html
layout/base/tests/bug1637476-3-ref.html
layout/base/tests/bug1637476-3.html
layout/base/tests/mochitest.ini
layout/base/tests/test_reftests_with_caret.html
layout/forms/nsTextControlFrame.cpp
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-1-ref.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Reference: Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  requestAnimationFrame(function() {
+    requestAnimationFrame(function() {
+      document.documentElement.removeAttribute("class");
+    });
+  });
+});
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-1.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input::placeholder {
+  background: white;
+  opacity: 1;
+}
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input placeholder="&nbsp;">
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  setTimeout(function(){document.documentElement.removeAttribute("class");}, 0);
+});
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-2-ref.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Reference: Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input type=password>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  requestAnimationFrame(function() {
+    requestAnimationFrame(function() {
+      document.documentElement.removeAttribute("class");
+    });
+  });
+});
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-2.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input::placeholder {
+  background: white;
+  opacity: 1;
+}
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input type=password placeholder="&nbsp;">
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  setTimeout(function(){document.documentElement.removeAttribute("class");}, 0);
+});
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-3-ref.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Reference: Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input type=number>
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  requestAnimationFrame(function() {
+    requestAnimationFrame(function() {
+      document.documentElement.removeAttribute("class");
+    });
+  });
+});
+</script>
new file mode 100644
--- /dev/null
+++ b/layout/base/tests/bug1637476-3.html
@@ -0,0 +1,24 @@
+<!doctype html>
+<html class="reftest-wait">
+<title>Caret is correctly painted with placeholder opacity:1</title>
+<script src="/tests/SimpleTest/SimpleTest.js"></script>
+<style>
+input::placeholder {
+  background: white;
+  opacity: 1;
+}
+input {
+  -webkit-appearance:none;  /* to avoid bug 1637804 */
+  /* reset any UA styles and use colors that won't trigger anti-aliasing issues */
+  background: white;
+  border: 1px solid black;
+  outline: 1px solid black;
+}
+</style>
+<input type=number placeholder="&nbsp;">
+<script>
+SimpleTest.waitForFocus(function() {
+  document.querySelector('input').focus();
+  setTimeout(function(){document.documentElement.removeAttribute("class");}, 0);
+});
+</script>
--- a/layout/base/tests/mochitest.ini
+++ b/layout/base/tests/mochitest.ini
@@ -358,16 +358,22 @@ support-files =
   bug1611661-ref.html
   bug1634543-1.html
   bug1634543-1-ref.html
   bug1634543-2.html
   bug1634543-3.html
   bug1634543-4.html
   bug1634743-1.html
   bug1634743-1-ref.html
+  bug1637476-1.html
+  bug1637476-1-ref.html
+  bug1637476-2.html
+  bug1637476-2-ref.html
+  bug1637476-3.html
+  bug1637476-3-ref.html
   image_rgrg-256x256.png
   input-invalid-ref.html
   input-maxlength-invalid-change.html
   input-maxlength-ui-invalid-change.html
   input-maxlength-ui-valid-change.html
   input-maxlength-valid-before-change.html
   input-maxlength-valid-change.html
   input-minlength-invalid-change.html
--- a/layout/base/tests/test_reftests_with_caret.html
+++ b/layout/base/tests/test_reftests_with_caret.html
@@ -266,16 +266,19 @@ var tests = [
     [ 'bug1634543-1.html' , 'bug1634543-1-ref.html' ] ,
     [ 'bug1634543-2.html' , 'bug1634543-1-ref.html' ] ,
     // TODO(emilio): This fails because nsInlineFrame::GetCaretBaseline doesn't
     // return one line-height for an empty inline, and it probably should..
     // [ 'bug1634543-3.html' , 'bug1634543-1-ref.html' ] ,
     [ 'bug1634543-4.html' , 'bug1634543-1-ref.html' ] ,
     // Caret + line-height + pseudo-element only.
     [ 'bug1634743-1.html' , 'bug1634743-1-ref.html' ] ,
+    [ 'bug1637476-1.html' , 'bug1637476-1-ref.html' ] ,
+    [ 'bug1637476-2.html' , 'bug1637476-2-ref.html' ] ,
+    [ 'bug1637476-3.html' , 'bug1637476-3-ref.html' ] ,
     function() {SpecialPowers.pushPrefEnv({'clear': [['layout.accessiblecaret.enabled_on_touch']]}, nextTest);} ,
     function() {SpecialPowers.pushPrefEnv({'set': [['accessibility.browsewithcaret', true]]}, nextTest);} ,
     [ 'bug1529492-1.html' , 'bug1529492-1-ref.html' ] ,
     function() {SpecialPowers.pushPrefEnv({'clear': [['accessibility.browsewithcaret']]}, nextTest);} ,
 ];
 
 if (!navigator.appVersion.includes("Android")) {
   tests.push([ 'bug512295-1.html' , 'bug512295-1-ref.html' ]);
--- a/layout/forms/nsTextControlFrame.cpp
+++ b/layout/forms/nsTextControlFrame.cpp
@@ -1285,39 +1285,54 @@ void nsTextControlFrame::BuildDisplayLis
   DisplayBorderBackgroundOutline(aBuilder, aLists);
 
   // Redirect all lists to the Content list so that nothing can escape, ie
   // opacity creating stacking contexts that then get sorted with stacking
   // contexts external to us.
   nsDisplayList* content = aLists.Content();
   nsDisplayListSet set(content, content, content, content, content, content);
 
-  for (nsIFrame* kid = mFrames.FirstChild(); kid; kid = kid->GetNextSibling()) {
+  // Clip the placeholder and preview text to the root box, so that it doesn't,
+  // e.g., overlay with our <input type="number"> spin buttons.
+  //
+  // For other input types, this will be a noop since we size the root via
+  // ReflowTextControlChild, which sets the same available space for all
+  // children.
+  auto clipToRoot = [&](Maybe<DisplayListClipState::AutoSaveRestore> aClip) {
+    if (mRootNode) {
+      if (auto* root = mRootNode->GetPrimaryFrame()) {
+        aClip.emplace(aBuilder);
+        nsRect rootBox(aBuilder->ToReferenceFrame(root), root->GetSize());
+        aClip->ClipContentDescendants(rootBox);
+      }
+    }
+  };
+
+  // We build the ::placeholder first so that it renders below mRootNode which
+  // draws the caret and we always want that on top (bug 1637476).
+  if (mPlaceholderDiv && control->GetPlaceholderVisibility() &&
+      mPlaceholderDiv->GetPrimaryFrame()) {
+    Maybe<DisplayListClipState::AutoSaveRestore> overlayTextClip;
+    clipToRoot(overlayTextClip);
+    auto* kid = mPlaceholderDiv->GetPrimaryFrame();
+    MOZ_ASSERT(kid->GetParent() == this);
+    BuildDisplayListForChild(aBuilder, kid, set, 0);
+  }
+
+  for (auto* kid : mFrames) {
     nsIContent* kidContent = kid->GetContent();
     Maybe<DisplayListClipState::AutoSaveRestore> overlayTextClip;
-    if (kidContent == mPlaceholderDiv && !control->GetPlaceholderVisibility()) {
-      continue;
+    if (kidContent == mPlaceholderDiv) {
+      continue;  // we handled mPlaceholderDiv explicitly above
     }
     if (kidContent == mPreviewDiv && !control->GetPreviewVisibility()) {
       continue;
     }
-    // Clip the preview text to the root box, so that it doesn't, e.g., overlay
-    // with our <input type="number"> spin buttons.
-    //
-    // For other input types, this will be a noop since we size the root via
-    // ReflowTextControlChild, which sets the same available space for all
-    // children.
-    if (kidContent == mPlaceholderDiv || kidContent == mPreviewDiv) {
-      if (mRootNode) {
-        if (auto* root = mRootNode->GetPrimaryFrame()) {
-          overlayTextClip.emplace(aBuilder);
-          nsRect rootBox(aBuilder->ToReferenceFrame(root), root->GetSize());
-          overlayTextClip->ClipContentDescendants(rootBox);
-        }
-      }
+    if (kidContent == mPreviewDiv) {
+      clipToRoot(overlayTextClip);
     }
     BuildDisplayListForChild(aBuilder, kid, set, 0);
   }
 }
 
 NS_IMETHODIMP
 nsTextControlFrame::EditorInitializer::Run() {
   if (!mFrame) {