Bug 1707070 - Tweak text/number/search control anonymous tree DOM / paint order. r=dholbert

So that we hit-test the spinners even if they overlap with the editor
root's padding.

Differential Revision: https://phabricator.services.mozilla.com/D113251

dom/html/test/forms/test_input_number_mouse_events.html

@@ -12,9 +12,11 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=935501
   <meta name="viewport" content="width=device-width, initial-scale=1.0">
   <style>
     input {
-      margin: 0 ! important;
-      border: 0 ! important;
-      padding: 0 ! important;
+      margin: 0;
+      border: 0;
+      padding: 0;
+      width: 200px;
+      box-sizing: border-box;
     }
   </style>
 </head>
@@ -25,7 +27,7 @@ https://bugzilla.mozilla.org/show_bug.cgi?id=935501
   <input id="input" type="number">
 </div>
 <pre id="test">
-<script type="application/javascript">
+<script>
 
 /**
  * Test for Bug 935501
@@ -120,6 +122,24 @@ function test() {
   is(input.value, "1", "Test that preventDefault() works for click on spin-down button");
   input.removeEventListener("mousedown", preventDefault);
 
+  // Test for bug 1707070.
+  input.style.paddingRight = "30px";
+  input.getBoundingClientRect(); // flush layout
+
+  input.value = 0;
+  synthesizeMouse(input, SPIN_UP_X - 30, SPIN_UP_Y, { type: "mousedown" });
+  is(input.value, "1", "Spinner down works on with padding (mousedown)");
+  synthesizeMouse(input, SPIN_UP_X - 30, SPIN_UP_Y, { type: "mouseup" });
+  is(input.value, "1", "Spinner down works with padding (mouseup)");
+
+  synthesizeMouse(input, SPIN_DOWN_X - 30, SPIN_DOWN_Y, { type: "mousedown" });
+  is(input.value, "0", "Spinner works with padding (mousedown)");
+  synthesizeMouse(input, SPIN_DOWN_X - 30, SPIN_DOWN_Y, { type: "mouseup" });
+  is(input.value, "0", "Spinner works with padding (mouseup)");
+
+  input.style.paddingRight = "";
+  input.getBoundingClientRect(); // flush layout
+
   // Run the spin tests:
   runNextSpinTest();
 }

layout/forms/nsNumberControlFrame.cpp

@@ -17,6 +17,7 @@
 #include "nsContentUtils.h"
 #include "nsContentCreatorFunctions.h"
 #include "nsCSSPseudoElements.h"
+#include "nsLayoutUtils.h"
 
 #ifdef ACCESSIBILITY
 #  include "mozilla/a11y/AccTypes.h"
@@ -53,15 +54,17 @@ nsresult nsNumberControlFrame::CreateAnonymousContent(
   // follows:
   //
   // input
+  //   div    - placeholder
+  //   div    - preview div
+  //   div    - editor root
   //   div    - spin box wrapping up/down arrow buttons
   //     div  - spin up (up arrow button)
   //     div  - spin down (down arrow button)
-  //   div    - editor root
-  //   div    - placeholder
-  //   div    - preview div
   //
-  // If you change this, be careful to change the destruction order in
-  // nsNumberControlFrame::DestroyFrom.
+  // If you change this, be careful to change the order of stuff returned in
+  // AppendAnonymousContentTo.
+
+  nsTextControlFrame::CreateAnonymousContent(aElements);
 
   // The author has elected to hide the spinner by setting this
   // -moz-appearance. We will reframe if it changes.
@@ -75,13 +78,9 @@ nsresult nsNumberControlFrame::CreateAnonymousContent(
     // Create the ::-moz-number-spin-down pseudo-element:
     mSpinDown = MakeAnonElement(PseudoStyleType::mozNumberSpinDown, mSpinBox);
 
-    // It's important that this goes first, so that reflow can know our size for
-    // the rest of the children.
     aElements.AppendElement(mSpinBox);
   }
 
-  nsTextControlFrame::CreateAnonymousContent(aElements);
-
   return NS_OK;
 }
 
@@ -164,10 +163,10 @@ bool nsNumberControlFrame::SpinnerDownButtonIsDepressed() const {
 
 void nsNumberControlFrame::AppendAnonymousContentTo(
     nsTArray<nsIContent*>& aElements, uint32_t aFilter) {
+  nsTextControlFrame::AppendAnonymousContentTo(aElements, aFilter);
   if (mSpinBox) {
     aElements.AppendElement(mSpinBox);
   }
-  nsTextControlFrame::AppendAnonymousContentTo(aElements, aFilter);
 }
 
 #ifdef ACCESSIBILITY

layout/forms/nsSearchControlFrame.cpp

@@ -51,34 +51,34 @@ nsresult nsSearchControlFrame::CreateAnonymousContent(
   // follows:
   //
   // input
-  //   button - clear button
-  //   div    - editor root
   //   div    - placeholder
   //   div    - preview div
+  //   div    - editor root
+  //   button - clear button
   //
-  // If you change this, be careful to change the destruction order in
-  // nsSearchControlFrame::DestroyFrom.
+  // If you change this, be careful to change the order of stuff in
+  // AppendAnonymousContentTo.
+
+  nsTextControlFrame::CreateAnonymousContent(aElements);
 
   // Create the ::-moz-search-clear-button pseudo-element:
   mClearButton = MakeAnonElement(PseudoStyleType::mozSearchClearButton, nullptr,
                                  nsGkAtoms::button);
 
-  aElements.AppendElement(mClearButton);
-
-  nsTextControlFrame::CreateAnonymousContent(aElements);
-
   // Update clear button visibility based on value
   UpdateClearButtonState();
 
+  aElements.AppendElement(mClearButton);
+
   return NS_OK;
 }
 
 void nsSearchControlFrame::AppendAnonymousContentTo(
     nsTArray<nsIContent*>& aElements, uint32_t aFilter) {
+  nsTextControlFrame::AppendAnonymousContentTo(aElements, aFilter);
   if (mClearButton) {
     aElements.AppendElement(mClearButton);
   }
-  nsTextControlFrame::AppendAnonymousContentTo(aElements, aFilter);
 }
 
 void nsSearchControlFrame::UpdateClearButtonState() {

layout/forms/nsTextControlFrame.cpp

@@ -405,7 +405,6 @@ nsresult nsTextControlFrame::CreateAnonymousContent(
     return rv;
   }
 
-  aElements.AppendElement(mRootNode);
   CreatePlaceholderIfNeeded();
   if (mPlaceholderDiv) {
     aElements.AppendElement(mPlaceholderDiv);
@@ -415,6 +414,10 @@ nsresult nsTextControlFrame::CreateAnonymousContent(
     aElements.AppendElement(mPreviewDiv);
   }
 
+  // NOTE(emilio): We want the root node always after the placeholder so that
+  // background on the placeholder doesn't obscure the caret.
+  aElements.AppendElement(mRootNode);
+
   rv = UpdateValueDisplay(false);
   NS_ENSURE_SUCCESS(rv, rv);
 
@@ -516,8 +519,6 @@ void nsTextControlFrame::CreatePreviewIfNeeded() {
 
 void nsTextControlFrame::AppendAnonymousContentTo(
     nsTArray<nsIContent*>& aElements, uint32_t aFilter) {
-  aElements.AppendElement(mRootNode);
-
   if (mPlaceholderDiv && !(aFilter & nsIContent::eSkipPlaceholderContent)) {
     aElements.AppendElement(mPlaceholderDiv);
   }
@@ -525,6 +526,8 @@ void nsTextControlFrame::AppendAnonymousContentTo(
   if (mPreviewDiv) {
     aElements.AppendElement(mPreviewDiv);
   }
+
+  aElements.AppendElement(mRootNode);
 }
 
 nscoord nsTextControlFrame::GetPrefISize(gfxContext* aRenderingContext) {
@@ -607,6 +610,12 @@ Maybe<nscoord> nsTextControlFrame::ComputeBaseline(
               aReflowInput.ComputedLogicalBorderPadding(wm).BStart(wm));
 }
 
+static bool IsButtonBox(const nsIFrame* aFrame) {
+  auto pseudoType = aFrame->Style()->GetPseudoType();
+  return pseudoType == PseudoStyleType::mozNumberSpinBox ||
+         pseudoType == PseudoStyleType::mozSearchClearButton;
+}
+
 void nsTextControlFrame::Reflow(nsPresContext* aPresContext,
                                 ReflowOutput& aDesiredSize,
                                 const ReflowInput& aReflowInput,
@@ -631,12 +640,32 @@ void nsTextControlFrame::Reflow(nsPresContext* aPresContext,
 
   // overflow handling
   aDesiredSize.SetOverflowAreasToDesiredBounds();
+
+  nsIFrame* buttonBox = [&]() -> nsIFrame* {
+    nsIFrame* last = mFrames.LastChild();
+    if (!last || !IsButtonBox(last)) {
+      return nullptr;
+    }
+    return last;
+  }();
+
+  // Reflow the button box first, so that we can use its size for the other
+  // frames.
+  nscoord buttonBoxISize = 0;
+  if (buttonBox) {
+    ReflowTextControlChild(buttonBox, aPresContext, aReflowInput, aStatus,
+                           aDesiredSize, buttonBoxISize);
+  }
+
   // perform reflow on all kids
   nsIFrame* kid = mFrames.FirstChild();
-  nscoord buttonBoxISize = 0;
   while (kid) {
-    ReflowTextControlChild(kid, aPresContext, aReflowInput, aStatus,
-                           aDesiredSize, buttonBoxISize);
+    if (kid != buttonBox) {
+      MOZ_ASSERT(!IsButtonBox(kid),
+                 "Should only have one button box, and should be last");
+      ReflowTextControlChild(kid, aPresContext, aReflowInput, aStatus,
+                             aDesiredSize, buttonBoxISize);
+    }
     kid = kid->GetNextSibling();
   }
 
@@ -657,9 +686,7 @@ void nsTextControlFrame::ReflowTextControlChild(
   LogicalSize availSize = aReflowInput.ComputedSizeWithPadding(wm);
   availSize.BSize(wm) = NS_UNCONSTRAINEDSIZE;
 
-  const bool isButtonBox =
-      aKid->Style()->GetPseudoType() == PseudoStyleType::mozNumberSpinBox ||
-      aKid->Style()->GetPseudoType() == PseudoStyleType::mozSearchClearButton;
+  bool isButtonBox = IsButtonBox(aKid);
 
   ReflowInput kidReflowInput(aPresContext, aReflowInput, aKid, availSize,
                              Nothing(), ReflowInput::InitFlag::CallerWillInit);
@@ -1270,12 +1297,6 @@ nsresult nsTextControlFrame::PeekOffset(nsPeekOffsetStruct* aPos) {
 
 void nsTextControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
                                           const nsDisplayListSet& aLists) {
-  /*
-   * The implementation of this method is equivalent as:
-   * nsContainerFrame::BuildDisplayList()
-   * with the difference that we filter-out the placeholder frame when it
-   * should not be visible.
-   */
   DO_GLOBAL_REFLOW_COUNT_DSP("nsTextControlFrame");
 
   DisplayBorderBackgroundOutline(aBuilder, aLists);
@@ -1286,20 +1307,7 @@ void nsTextControlFrame::BuildDisplayList(nsDisplayListBuilder* aBuilder,
   nsDisplayList* content = aLists.Content();
   nsDisplayListSet set(content, content, content, content, content, content);
 
-  // We build the ::placeholder first so that it renders below mRootNode which
-  // draws the caret and we always want that on top (bug 1637476).
-  //
-  // TODO(emilio): We should consider just changing the DOM order instead.
-  if (mPlaceholderDiv && mPlaceholderDiv->GetPrimaryFrame()) {
-    auto* kid = mPlaceholderDiv->GetPrimaryFrame();
-    MOZ_ASSERT(kid->GetParent() == this);
-    BuildDisplayListForChild(aBuilder, kid, set);
-  }
-
   for (auto* kid : mFrames) {
-    if (kid->GetContent() == mPlaceholderDiv) {
-      continue;  // Handled above already.
-    }
     BuildDisplayListForChild(aBuilder, kid, set);
   }
 }

layout/style/res/forms.css

@@ -147,6 +147,7 @@ textarea > scrollbar {
 ::-moz-text-control-preview {
   overflow: auto;
   border: 0;
+  /* This is necessary to make overflow-clip-box work */
   padding: inherit;
   margin: 0;
   text-decoration: inherit;
@@ -204,10 +205,12 @@ textarea::-moz-text-control-editing-root {
   overflow: hidden;
 
   /*
-   * The placeholder or preview should be ignored by pointer otherwise, we might have some
-   * unexpected behavior like the resize handle not being selectable.
+   * The placeholder or preview should be ignored by pointer / selection / etc.
+   * Otherwise, we might have some unexpected behavior like the resize handle
+   * not being selectable.
    */
   pointer-events: none;
+  user-select: none;
 }
 
 ::placeholder {