Bug 1272983 part 2: Use more forgiving margin/padding getters in nsButtonFrameRenderer, for better behavior when percent or auto values are encountered. r=dbaron
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 20 May 2016 15:05:33 -0700
changeset 369241 c76ec44e0c23aba254c49e51843a8fc0d7e726f4
parent 369240 66e13bb5f4c0f8437a280b9d603daadea89dba6c
child 369242 7cab985e78c8b9afbe2b0a7a04bb123894ef44b2
push id18807
push userdholbert@mozilla.com
push dateFri, 20 May 2016 22:18:21 +0000
reviewersdbaron
bugs1272983
milestone49.0a1
Bug 1272983 part 2: Use more forgiving margin/padding getters in nsButtonFrameRenderer, for better behavior when percent or auto values are encountered. r=dbaron MozReview-Commit-ID: 4vIehNg2vND
layout/forms/nsButtonFrameRenderer.cpp
layout/reftests/forms/button/focus-area-percent-units-1-ref.html
layout/reftests/forms/button/focus-area-percent-units-1.html
layout/reftests/forms/button/focus-area-percent-units-2-ref.html
layout/reftests/forms/button/focus-area-percent-units-2.html
layout/reftests/forms/button/reftest.list
--- a/layout/forms/nsButtonFrameRenderer.cpp
+++ b/layout/forms/nsButtonFrameRenderer.cpp
@@ -381,17 +381,17 @@ nsButtonFrameRenderer::GetButtonInnerFoc
 
 
 nsMargin
 nsButtonFrameRenderer::GetButtonOuterFocusBorderAndPadding()
 {
   nsMargin result(0,0,0,0);
 
   if (mOuterFocusStyle) {
-    mOuterFocusStyle->StylePadding()->GetPaddingNoPercentage(result);
+    mOuterFocusStyle->StylePadding()->GetPadding(result);
     result += mOuterFocusStyle->StyleBorder()->GetComputedBorder();
   }
 
   return result;
 }
 
 nsMargin
 nsButtonFrameRenderer::GetButtonBorderAndPadding()
@@ -404,29 +404,29 @@ nsButtonFrameRenderer::GetButtonBorderAn
  */
 nsMargin
 nsButtonFrameRenderer::GetButtonInnerFocusMargin()
 {
   nsMargin innerFocusMargin(0,0,0,0);
 
   if (mInnerFocusStyle) {
     const nsStyleMargin* margin = mInnerFocusStyle->StyleMargin();
-    margin->GetMarginNoPercentage(innerFocusMargin);
+    margin->GetMargin(innerFocusMargin);
   }
 
   return innerFocusMargin;
 }
 
 nsMargin
 nsButtonFrameRenderer::GetButtonInnerFocusBorderAndPadding()
 {
   nsMargin result(0,0,0,0);
 
   if (mInnerFocusStyle) {
-    mInnerFocusStyle->StylePadding()->GetPaddingNoPercentage(result);
+    mInnerFocusStyle->StylePadding()->GetPadding(result);
     result += mInnerFocusStyle->StyleBorder()->GetComputedBorder();
   }
 
   return result;
 }
 
 // gets all the focus borders and padding that will be added to the regular border
 nsMargin
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/focus-area-percent-units-1-ref.html
@@ -0,0 +1,52 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<head>
+  <title>Reference case for bug 1272983</title>
+  <style>
+    /* Set explicit font size so that em units are predictable: */
+    body, button { font: 10px sans-serif; }
+
+    /* Set margin to 0 for all cases. In the first 6, that's how we expect
+       the testcase to render; and in the 7th and 8th, our reference margin
+       will be applied via a child div instead of via the pseudo-element. */
+    button.mfi1::-moz-focus-inner,
+    button.mfi2::-moz-focus-inner,
+    button.mfi3::-moz-focus-inner,
+    button.mfi4::-moz-focus-inner,
+    button.mfi5::-moz-focus-inner,
+    button.mfi6::-moz-focus-inner,
+    button.mfi7::-moz-focus-inner,
+    button.mfi8::-moz-focus-inner { margin: 0; }
+
+    /* Use an explicit div instead of pseudo-element, for reference case's
+       version of margin values that we actually expect to take effect: */
+    button.mfi7 > div { margin: 10px; }
+    button.mfi8 > div { margin: 20px; /* = 2em * 20px/em */ }
+  </style>
+</head>
+<body>
+  <button class="mfi1">mfi1</button>
+  <button class="mfi2">mfi2</button>
+  <button class="mfi3">mfi3</button>
+  <button class="mfi4">mfi4</button>
+  <br>
+  <button class="mfi5">mfi5</button>
+  <button class="mfi6">mfi6</button>
+  <button class="mfi7"><div>mfi7</div></button>
+  <button class="mfi8"><div>mfi8</div></button>
+  <br>
+
+  <button class="mfo1">mfo1</button>
+  <button class="mfo2">mfo2</button>
+  <button class="mfo3">mfo3</button>
+  <button class="mfo4">mfo4</button>
+  <br>
+  <button class="mfo5">mfo5</button>
+  <button class="mfo6">mfo6</button>
+  <button class="mfo7">mfo7</button>
+  <button class="mfo8">mfo8</button>
+  <br>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/focus-area-percent-units-1.html
@@ -0,0 +1,62 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<!-- The ::-moz-focus-inner & ::-moz-focus-outer button pseudo-elements only
+     support CSS "margin" values that contain absolute lengths.  Any percent or
+     "auto" margin values will simply make the margin collapse to zero.
+     This test verifies that this is indeed what happens (not anything worse).
+-->
+<head>
+  <title>Testcase for bug 1272983</title>
+  <style>
+    /* Set explicit font size so that em units are predictable: */
+    body, button { font: 10px sans-serif; }
+
+    /* Testing percent and auto margin values on "-moz-focus-inner": */
+    button.mfi1::-moz-focus-inner { margin: 50%; }
+    button.mfi2::-moz-focus-inner { margin: 50% 10px; }
+    button.mfi3::-moz-focus-inner { margin: 10px 50%; }
+    button.mfi4::-moz-focus-inner { margin: auto; }
+    button.mfi5::-moz-focus-inner { margin: auto 10px; }
+    button.mfi6::-moz-focus-inner { margin: 10px auto; }
+    button.mfi7::-moz-focus-inner { margin: 10px; }
+    button.mfi8::-moz-focus-inner { margin: 2em; }
+
+    /* Testing percent and auto margin values on "-moz-focus-outer":
+       (just for completeness -- really, 'margin' has no effect on
+       the behavior of -moz-focus-outer) */
+    button.mfo1::-moz-focus-outer { margin: 50%; }
+    button.mfo2::-moz-focus-outer { margin: 50% 10px; }
+    button.mfo3::-moz-focus-outer { margin: 10px 50%; }
+    button.mfo4::-moz-focus-outer { margin: auto; }
+    button.mfo5::-moz-focus-outer { margin: auto 10px; }
+    button.mfo6::-moz-focus-outer { margin: 10px auto; }
+    button.mfo7::-moz-focus-outer { margin: 10px; }
+    button.mfo8::-moz-focus-outer { margin: 2em; }
+  </style>
+</head>
+<body>
+  <button class="mfi1">mfi1</button>
+  <button class="mfi2">mfi2</button>
+  <button class="mfi3">mfi3</button>
+  <button class="mfi4">mfi4</button>
+  <br>
+  <button class="mfi5">mfi5</button>
+  <button class="mfi6">mfi6</button>
+  <button class="mfi7">mfi7</button>
+  <button class="mfi8">mfi8</button>
+  <br>
+
+  <button class="mfo1">mfo1</button>
+  <button class="mfo2">mfo2</button>
+  <button class="mfo3">mfo3</button>
+  <button class="mfo4">mfo4</button>
+  <br>
+  <button class="mfo5">mfo5</button>
+  <button class="mfo6">mfo6</button>
+  <button class="mfo7">mfo7</button>
+  <button class="mfo8">mfo8</button>
+  <br>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/focus-area-percent-units-2-ref.html
@@ -0,0 +1,68 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<head>
+  <title>Reference case for bug 1272983</title>
+  <style>
+    /* Set explicit font size so that em units are predictable: */
+    body, button { font: 10px sans-serif; }
+
+    /* Add outline to help visualize the padding outside of buttons: */
+    button { outline: 1px solid black; }
+
+    /* Set padding to 0 for all cases. In the first 3, that's how we expect
+       the testcase to render; and in the 4th and 5th, our reference padding
+       will be applied via a child div instead of via the pseudo-element. */
+    button.mfi1::-moz-focus-inner,
+    button.mfi2::-moz-focus-inner,
+    button.mfi3::-moz-focus-inner,
+    button.mfi4::-moz-focus-inner,
+    button.mfi5::-moz-focus-inner { padding: 0; }
+
+    /* Use an explicit div instead of pseudo-element, for reference case's
+       version of padding values that we actually expect to take effect: */
+    button.mfi4 > div { padding: 10px; }
+    button.mfi5 > div { padding: 2em; }
+
+    /* As above, set padding to 0 for all cases: */
+    button.mfo1::-moz-focus-outer,
+    button.mfo2::-moz-focus-outer,
+    button.mfo3::-moz-focus-outer,
+    button.mfo4::-moz-focus-outer,
+    button.mfo5::-moz-focus-outer { padding: 0; }
+
+    /* To make reference for -moz-focus-outer padding that we expect to
+       take effect, we'll put the padding on a wrapper-div (and bump the
+       button's outline to that div). */
+    div.mfo4-wrapper { padding: 10px; }
+    div.mfo5-wrapper { padding: 20px; /* = 2em * 10px/em */ }
+
+    button.mfo4,
+    button.mfo5 { outline: none; }
+    div.mfo4-wrapper,
+    div.mfo5-wrapper { display: inline-block; outline: 1px solid black; }
+  </style>
+</head>
+<body>
+  <button class="mfi1">mfi1</button>
+  <button class="mfi2">mfi2</button>
+  <button class="mfi3">mfi3</button>
+  <br>
+  <button class="mfi4"><div>mfi4</div></button>
+  <button class="mfi5"><div>mfi5</div></button>
+  <br>
+
+  <button class="mfo1">mfo1</button>
+  <button class="mfo2">mfo2</button>
+  <button class="mfo3">mfo3</button>
+  <br>
+  <div class="mfo4-wrapper">
+    <button class="mfo4">mfo4</button>
+  </div>
+  <div class="mfo5-wrapper">
+    <button class="mfo5">mfo5</button>
+  </div>
+  <br>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/focus-area-percent-units-2.html
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<!-- The ::-moz-focus-inner & ::-moz-focus-outer button pseudo-elements only
+     support CSS "padding" values that contain absolute lengths.  Any percent
+     padding values will simply make the padding collapse to zero.
+     This test verifies that this is indeed what happens (not anything worse).
+-->
+<head>
+  <title>Testcase for bug 1272983</title>
+  <style>
+    /* Set explicit font size so that em units are predictable: */
+    body, button { font: 10px sans-serif; }
+
+    /* Add outline to help visualize the padding outside of buttons: */
+    button { outline: 1px solid black; }
+
+    /* Testing percent and auto padding values on "-moz-focus-inner": */
+    button.mfi1::-moz-focus-inner { padding: 50%; }
+    button.mfi2::-moz-focus-inner { padding: 50% 10px; }
+    button.mfi3::-moz-focus-inner { padding: 10px 50%; }
+    button.mfi4::-moz-focus-inner { padding: 10px; }
+    button.mfi5::-moz-focus-inner { padding: 2em; }
+
+    /* Testing percent and auto padding values on "-moz-focus-outer": */
+    button.mfo1::-moz-focus-outer { padding: 50%; }
+    button.mfo2::-moz-focus-outer { padding: 50% 10px; }
+    button.mfo3::-moz-focus-outer { padding: 10px 50%; }
+    button.mfo4::-moz-focus-outer { padding: 10px; }
+    button.mfo5::-moz-focus-outer { padding: 2em; }
+  </style>
+</head>
+<body>
+  <button class="mfi1">mfi1</button>
+  <button class="mfi2">mfi2</button>
+  <button class="mfi3">mfi3</button>
+  <br>
+  <button class="mfi4">mfi4</button>
+  <button class="mfi5">mfi5</button>
+  <br>
+
+  <button class="mfo1">mfo1</button>
+  <button class="mfo2">mfo2</button>
+  <button class="mfo3">mfo3</button>
+  <br>
+  <button class="mfo4">mfo4</button>
+  <button class="mfo5">mfo5</button>
+  <br>
+</body>
+</html>
--- a/layout/reftests/forms/button/reftest.list
+++ b/layout/reftests/forms/button/reftest.list
@@ -1,10 +1,14 @@
 == first-letter-1.html first-letter-1-ref.html
 != first-letter-1.html first-letter-1-noref.html
+
+== focus-area-percent-units-1.html focus-area-percent-units-1-ref.html
+== focus-area-percent-units-2.html focus-area-percent-units-2-ref.html
+
 == max-height.html max-height-ref.html
 == min-height.html min-height-ref.html
 == overflow-areas-1.html overflow-areas-1-ref.html
 
 # The buttons in these tests have some fancy shading applied to their corners
 # on B2G, despite their "-moz-appearance: none; background: gray", so they
 # don't quite match the reference case's normal <div>. That's why they're fuzzy.
 fuzzy-if(B2G||Mulet||Android,125,20) == percent-height-child-1.html percent-height-child-1-ref.html # Initial mulet triage: parity with B2G/B2G Desktop