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 298271 c76ec44e0c23aba254c49e51843a8fc0d7e726f4
parent 298270 66e13bb5f4c0f8437a280b9d603daadea89dba6c
child 298272 7cab985e78c8b9afbe2b0a7a04bb123894ef44b2
push id30281
push usercbook@mozilla.com
push dateTue, 24 May 2016 12:54:02 +0000
treeherdermozilla-central@829d3be6ba64 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersdbaron
bugs1272983
milestone49.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 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