Bug 1261284: Don't include <button>'s children in overflow areas, if we know we're going to clip them when painting. r=mats
authorDaniel Holbert <dholbert@cs.stanford.edu>
Fri, 01 Apr 2016 20:36:41 -0700
changeset 291452 6b6889e28ca674258d9dc7551e45b27f336ecd1d
parent 291451 4678a49486b3fe0a75c9a8aabc14fd4e43ec4c85
child 291453 e7a1c3636af1263c50616514847b84f18e2563be
push id19656
push usergwagner@mozilla.com
push dateMon, 04 Apr 2016 13:43:23 +0000
treeherderb2g-inbound@e99061fde28a [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmats
bugs1261284
milestone48.0a1
Bug 1261284: Don't include <button>'s children in overflow areas, if we know we're going to clip them when painting. r=mats MozReview-Commit-ID: BG1tGEQjFI2
layout/forms/nsHTMLButtonControlFrame.cpp
layout/forms/nsHTMLButtonControlFrame.h
layout/reftests/forms/button/overflow-areas-1-ref.html
layout/reftests/forms/button/overflow-areas-1.html
layout/reftests/forms/button/reftest.list
--- a/layout/forms/nsHTMLButtonControlFrame.cpp
+++ b/layout/forms/nsHTMLButtonControlFrame.cpp
@@ -84,16 +84,21 @@ nsHTMLButtonControlFrame::HandleEvent(ns
     return NS_OK;
   }
 
   // mouse clicks are handled by content
   // we don't want our children to get any events. So just pass it to frame.
   return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus);
 }
 
+bool
+nsHTMLButtonControlFrame::ShouldClipPaintingToBorderBox()
+{
+  return IsInput() || StyleDisplay()->mOverflowX != NS_STYLE_OVERFLOW_VISIBLE;
+}
 
 void
 nsHTMLButtonControlFrame::BuildDisplayList(nsDisplayListBuilder*   aBuilder,
                                            const nsRect&           aDirtyRect,
                                            const nsDisplayListSet& aLists)
 {
   // Clip to our border area for event hit testing.
   Maybe<DisplayListClipState::AutoSaveRestore> eventClipState;
@@ -112,17 +117,17 @@ nsHTMLButtonControlFrame::BuildDisplayLi
   }
 
   nsDisplayListCollection set;
 
   // Do not allow the child subtree to receive events.
   if (!isForEventDelivery) {
     DisplayListClipState::AutoSaveRestore clipState(aBuilder);
 
-    if (IsInput() || StyleDisplay()->mOverflowX != NS_STYLE_OVERFLOW_VISIBLE) {
+    if (ShouldClipPaintingToBorderBox()) {
       nsMargin border = StyleBorder()->GetComputedBorder();
       nsRect rect(aBuilder->ToReferenceFrame(this), GetSize());
       rect.Deflate(border);
       nscoord radii[8];
       bool hasRadii = GetPaddingBoxBorderRadii(radii);
       clipState.ClipContainingBlockDescendants(rect, hasRadii ? radii : nullptr);
     }
 
@@ -209,17 +214,21 @@ nsHTMLButtonControlFrame::Reflow(nsPresC
   // !NS_SUBTREE_DIRTY(firstKid).
   // We'd need to cache our ascent for that, of course.
 
   // Reflow the contents of the button.
   // (This populates our aDesiredSize, too.)
   ReflowButtonContents(aPresContext, aDesiredSize,
                        aReflowState, firstKid);
 
-  ConsiderChildOverflow(aDesiredSize.mOverflowAreas, firstKid);
+  if (!ShouldClipPaintingToBorderBox()) {
+    ConsiderChildOverflow(aDesiredSize.mOverflowAreas, firstKid);
+  }
+  // else, we ignore child overflow -- anything that overflows beyond our
+  // own border-box will get clipped when painting.
 
   aStatus = NS_FRAME_COMPLETE;
   FinishReflowWithAbsoluteFrames(aPresContext, aDesiredSize,
                                  aReflowState, aStatus);
 
   // We're always complete and we don't support overflow containers
   // so we shouldn't have a next-in-flow ever.
   aStatus = NS_FRAME_COMPLETE;
--- a/layout/forms/nsHTMLButtonControlFrame.h
+++ b/layout/forms/nsHTMLButtonControlFrame.h
@@ -87,16 +87,22 @@ public:
   virtual bool IsFrameOfType(uint32_t aFlags) const override
   {
     return nsContainerFrame::IsFrameOfType(aFlags &
       ~(nsIFrame::eReplaced | nsIFrame::eReplacedContainsBlock));
   }
 
 protected:
   virtual bool IsInput() { return false; }
+
+  // Indicates whether we should clip our children's painting to our
+  // border-box (either because of "overflow" or because of legacy reasons
+  // about how <input>-flavored buttons work).
+  bool ShouldClipPaintingToBorderBox();
+
   // Reflows the button's sole child frame, and computes the desired size
   // of the button itself from the results.
   void ReflowButtonContents(nsPresContext* aPresContext,
                             nsHTMLReflowMetrics& aButtonDesiredSize,
                             const nsHTMLReflowState& aButtonReflowState,
                             nsIFrame* aFirstKid);
 
   nsButtonFrameRenderer mRenderer;
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/overflow-areas-1-ref.html
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<head>
+  <title>Reference case</title>
+  <style>
+    input, button {
+      border: 0;             /* Combined, these mean the gray area is the */
+      background: lightgray; /* border-box size. */
+
+      outline: 2px solid black; /* The outlined area is the overflow area. */
+      width: 1px;   /* (To attempt to trigger overflow) */
+
+      display: block;     /* Put each button on its own line, w/ some margin, */
+      margin-bottom: 5px; /* so that any overflow doesn't get stomped on.     */
+
+      font: 8px serif; /* (This just lets the testcase fit better on mobile.) */
+    }
+
+    .oh { overflow: hidden }
+  </style>
+</head>
+<body>
+  <!-- For the reference case, we just put "overflow:hidden" on everything. -->
+  <input class="oh" type="reset">
+  <input class="oh" type="submit">
+  <input class="oh" type="button" value="InputTypeButton">
+  <!-- ...with one exception: button with (default) overflow:visible.
+       Such buttons *do* actually allow their contents to overflow. -->
+  <button>ActualButton</button>
+
+  <input  class="oh" type="reset">
+  <input  class="oh" type="submit">
+  <input  class="oh" type="button" value="InputTypeButton">
+  <button class="oh">ActualButton</button>
+
+  <input  class="oh" type="reset">
+  <input  class="oh" type="submit">
+  <input  class="oh" type="button" value="InputTypeButton">
+  <button class="oh">ActualButton</button>
+
+  <input  class="oh" type="reset">
+  <input  class="oh" type="submit">
+  <input  class="oh" type="button" value="InputTypeButton">
+  <button class="oh">ActualButton</button>
+</body>
+</html>
new file mode 100644
--- /dev/null
+++ b/layout/reftests/forms/button/overflow-areas-1.html
@@ -0,0 +1,51 @@
+<!DOCTYPE html>
+<!-- Any copyright is dedicated to the Public Domain.
+   - http://creativecommons.org/publicdomain/zero/1.0/ -->
+<html>
+<!-- For buttons whose painting gets clipped to their border-box area,
+     we should *also* clip their overflow area (as exposed via 'outline').
+     This test exposes these areas so they can be visualized, and checks that
+     they match when we expect them to. -->
+<head>
+  <title>Testcase for bug 1261284</title>
+  <style>
+    input, button {
+      border: 0;             /* Combined, these mean the gray area is the */
+      background: lightgray; /* border-box size. */
+
+      outline: 2px solid black; /* The outlined area is the overflow area. */
+      width: 1px;   /* (To attempt to trigger overflow) */
+
+      display: block;     /* Put each button on its own line, w/ some margin, */
+      margin-bottom: 5px; /* so that any overflow doesn't get stomped on.     */
+
+      font: 8px serif; /* (This just lets the testcase fit better on mobile.) */
+    }
+
+    .oh { overflow: hidden }
+    .oa { overflow: auto }
+    .os { overflow: scroll }
+  </style>
+</head>
+<body>
+  <input type="reset">
+  <input type="submit">
+  <input type="button" value="InputTypeButton">
+  <button>ActualButton</button>
+
+  <input  class="oh" type="reset">
+  <input  class="oh" type="submit">
+  <input  class="oh" type="button" value="InputTypeButton">
+  <button class="oh">ActualButton</button>
+
+  <input  class="oa" type="reset">
+  <input  class="oa" type="submit">
+  <input  class="oa" type="button" value="InputTypeButton">
+  <button class="oa">ActualButton</button>
+
+  <input  class="os" type="reset">
+  <input  class="os" type="submit">
+  <input  class="os" type="button" value="InputTypeButton">
+  <button class="os">ActualButton</button>
+</body>
+</html>
--- a/layout/reftests/forms/button/reftest.list
+++ b/layout/reftests/forms/button/reftest.list
@@ -1,12 +1,13 @@
 == first-letter-1.html first-letter-1-ref.html
 != first-letter-1.html first-letter-1-noref.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
 pref(browser.display.focus_ring_width,1) fuzzy-if(B2G||Mulet||Android,125,80) == percent-height-child-2.html percent-height-child-2-ref.html # Initial mulet triage: parity with B2G/B2G Desktop
 fuzzy-if(B2G||Mulet||Android,125,20) == percent-width-child-1.html  percent-width-child-1-ref.html # Initial mulet triage: parity with B2G/B2G Desktop
 pref(browser.display.focus_ring_width,1) fuzzy-if(B2G||Mulet||Android,125,80) == percent-width-child-2.html  percent-width-child-2-ref.html # Initial mulet triage: parity with B2G/B2G Desktop