Bug 1624655 - Don't backplate over themed backgrounds. r=morgan
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 25 Mar 2020 15:45:21 +0000
changeset 520396 cb8f33fc316772581fc4a03e02571f483c79bdaa
parent 520395 73b4073fe12e1062ac6a2b64618c3c6a9fdb145c
child 520397 3532b2cd85cac8b4b0fd946b2f23415ebe060d7a
push id37249
push userdvarga@mozilla.com
push dateWed, 25 Mar 2020 21:39:06 +0000
treeherdermozilla-central@b3c3f7d0f044 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersmorgan
bugs1624655
milestone76.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 1624655 - Don't backplate over themed backgrounds. r=morgan data:text/html,<input type=submit> is white-on-white when depressed with backplating enabled on the GTK light theme, because even though the computed background-color is white, the actual button background is themed and the color is not white instead. This causes problems with the unthemed appearance anyway (<input type=submit style="-webkit-appearance: none"> and so on), but this is probably worth it anyway. Differential Revision: https://phabricator.services.mozilla.com/D68063
layout/generic/nsBlockFrame.cpp
layout/reftests/high-contrast/reftest.list
--- a/layout/generic/nsBlockFrame.cpp
+++ b/layout/generic/nsBlockFrame.cpp
@@ -215,29 +215,32 @@ static nsRect GetLineTextArea(nsLineBox*
 }
 
 /**
  * Starting with aFrame, iterates upward through parent frames and checks for
  * non-transparent background colors. If one is found, we use that as our
  * backplate color. Otheriwse, we use the default background color from
  * our high contrast theme.
  */
-static nscolor GetBackplateColor(nsIFrame* aFrame) {
+static Maybe<nscolor> GetBackplateColor(nsIFrame* aFrame) {
   for (nsIFrame* frame = aFrame; frame; frame = frame->GetParent()) {
+    if (frame->IsThemed()) {
+      return Nothing();
+    }
     auto* bg = frame->StyleBackground();
     if (bg->IsTransparent(frame)) {
       continue;
     }
     nscolor backgroundColor = bg->BackgroundColor(frame);
     if (NS_GET_A(backgroundColor) != 0) {
-      return backgroundColor;
+      return Some(backgroundColor);
     }
     break;
   }
-  return aFrame->PresContext()->DefaultBackgroundColor();
+  return Some(aFrame->PresContext()->DefaultBackgroundColor());
 }
 
 #ifdef DEBUG
 #  include "nsBlockDebugFlags.h"
 
 bool nsBlockFrame::gLamePaintMetrics;
 bool nsBlockFrame::gLameReflowMetrics;
 bool nsBlockFrame::gNoisy;
@@ -6941,42 +6944,47 @@ void nsBlockFrame::BuildDisplayList(nsDi
         (GetStateBits() & NS_FRAME_FORCE_DISPLAY_LIST_DESCEND_INTO) ||
         aBuilder->GetIncludeAllOutOfFlows();
 
     return descendAlways || aLineArea.Intersects(aBuilder->GetDirtyRect()) ||
            (ForceDescendIntoIfVisible() &&
             aLineArea.Intersects(aBuilder->GetVisibleRect()));
   };
 
-  // We'll try to draw an accessibility backplate behind text
-  // (to ensure it's readable over any possible background-images),
-  // if all of the following hold:
-  //    (A) the backplate feature is preffed on
-  //    (B) we are not honoring the document colors
-  const bool shouldDrawBackplate =
-      StaticPrefs::browser_display_permit_backplate() &&
-      !PresContext()->PrefSheetPrefs().mUseDocumentColors &&
-      !IsComboboxControlFrame();
+  Maybe<nscolor> backplateColor;
+
+  {
+    // We'll try to draw an accessibility backplate behind text (to ensure it's
+    // readable over any possible background-images), if all of the following
+    // hold:
+    //    (A) the backplate feature is preffed on
+    //    (B) we are not honoring the document colors
+    if (StaticPrefs::browser_display_permit_backplate() &&
+        !PresContext()->PrefSheetPrefs().mUseDocumentColors &&
+        !IsComboboxControlFrame()) {
+      backplateColor = GetBackplateColor(this);
+    }
+  }
 
   // Don't use the line cursor if we might have a descendant placeholder ...
   // it might skip lines that contain placeholders but don't themselves
   // intersect with the dirty area.
   // In particular, we really want to check ShouldDescendIntoFrame()
   // on all our child frames, but that might be expensive.  So we
   // approximate it by checking it on |this|; if it's true for any
   // frame in our child list, it's also true for |this|.
   // Also skip the cursor if we're creating text overflow markers,
   // since we need to know what line number we're up to in order
   // to generate unique display item keys.
   // Lastly, the cursor should be skipped if we're drawing
   // backplates behind text. When backplating we consider consecutive
   // runs of text as a whole, which requires we iterate through all lines
   // to find our backplate size.
   nsLineBox* cursor = (hasDescendantPlaceHolders || textOverflow.isSome() ||
-                       shouldDrawBackplate)
+                       backplateColor)
                           ? nullptr
                           : GetFirstLineContaining(aBuilder->GetDirtyRect().y);
   LineIterator line_end = LinesEnd();
 
   TextOverflow* textOverflowPtr = textOverflow.ptrOr(nullptr);
 
   if (cursor) {
     for (LineIterator line = mLines.begin(cursor); line != line_end; ++line) {
@@ -6996,20 +7004,16 @@ void nsBlockFrame::BuildDisplayList(nsDi
       }
     }
   } else {
     bool nonDecreasingYs = true;
     uint32_t lineCount = 0;
     nscoord lastY = INT32_MIN;
     nscoord lastYMost = INT32_MIN;
     nsRect curBackplateArea;
-    Maybe<nscolor> backplateColor;
-    if (shouldDrawBackplate) {
-      backplateColor = Some(GetBackplateColor(this));
-    }
     // A frame's display list cannot contain more than one copy of a
     // given display item unless the items are uniquely identifiable.
     // Because backplate occasionally requires multiple
     // SolidColor items, we use an index (backplateIndex) to maintain
     // uniqueness among them. Note this is a mapping of index to
     // item, and the mapping is stable even if the dirty rect changes.
     uint16_t backplateIndex = 0;
     for (LineIterator line = LinesBegin(); line != line_end; ++line) {
@@ -7020,35 +7024,34 @@ void nsBlockFrame::BuildDisplayList(nsDi
         DisplayLine(aBuilder, line, lineInLine, aLists, this, textOverflowPtr,
                     lineCount, depth, drawnLines);
       }
 
       if (!lineInLine && !curBackplateArea.IsEmpty()) {
         // If we have encountered a non-inline line but were previously
         // forming a backplate, we should add the backplate to the display
         // list as-is and render future backplates disjointly.
-        MOZ_ASSERT(shouldDrawBackplate,
+        MOZ_ASSERT(backplateColor,
                    "if this master switch is off, curBackplateArea "
                    "must be empty and we shouldn't get here");
         aLists.BorderBackground()->AppendNewToTop<nsDisplaySolidColor>(
             aBuilder, this, curBackplateArea, backplateColor.value(),
             backplateIndex);
         backplateIndex++;
 
         curBackplateArea = nsRect();
       }
 
       if (!lineArea.IsEmpty()) {
         if (lineArea.y < lastY || lineArea.YMost() < lastYMost) {
           nonDecreasingYs = false;
         }
         lastY = lineArea.y;
         lastYMost = lineArea.YMost();
-        if (lineInLine && shouldDrawBackplate &&
-            LineHasVisibleInlineContent(line)) {
+        if (lineInLine && backplateColor && LineHasVisibleInlineContent(line)) {
           nsRect lineBackplate = GetLineTextArea(line, aBuilder) +
                                  aBuilder->ToReferenceFrame(this);
           if (curBackplateArea.IsEmpty()) {
             curBackplateArea = lineBackplate;
           } else {
             curBackplateArea.OrWith(lineBackplate);
           }
         }
--- a/layout/reftests/high-contrast/reftest.list
+++ b/layout/reftests/high-contrast/reftest.list
@@ -1,28 +1,25 @@
 # This pref enables high-contrast mode in the testcases here:
 defaults test-pref(browser.display.document_color_use,2) test-pref(browser.display.permit_backplate,true)
 
 # There are several platform-specific fuzzy-if notations below due to
 # anti-aliasing, extra visual overflow, and slightly inacurate reftest
 # mocking. You can read more about the annotations and their specifics here:
 # https://bugzilla.mozilla.org/show_bug.cgi?id=1539212#c25
 
-# non-native-theme uses non-white background, so the white backplate shows up
-# as a difference with the reference.
-fails-if(!nativeThemePref) == backplate-select-001.html backplate-select-001-ref.html
+== backplate-select-001.html backplate-select-001-ref.html
 
 fuzzy-if(cocoaWidget,255-255,20-20) == backplate-bg-image-001.html backplate-bg-image-001-ref.html # bug 1577028
 fuzzy-if(cocoaWidget,255-255,20-20) == backplate-bg-image-002.html backplate-bg-image-002-ref.html
 fuzzy-if(cocoaWidget,255-255,4-4) == backplate-bg-image-003.html backplate-bg-image-003-ref.html
 fuzzy-if(cocoaWidget,255-255,20-20) == backplate-bg-image-004.html backplate-bg-image-004-ref.html
 fuzzy-if(cocoaWidget,255-255,80-80) == backplate-bg-image-005.html backplate-bg-image-005-ref.html
 
-# Seems like this test should probably be rewritten...
-fuzzy-if(cocoaWidget,255-255,2912-2912) fuzzy-if(gtkWidget,13-13,3200-3200) fuzzy-if(winWidget,15-26,3000-3200) fuzzy-if(!nativeThemePref,24-24,3200-3202) == backplate-bg-image-006.html backplate-bg-image-006-ref.html
+fuzzy-if(cocoaWidget,255-255,40-40) == backplate-bg-image-006.html backplate-bg-image-006-ref.html
 
 fuzzy-if(cocoaWidget,255-255,40-40) == backplate-bg-image-007.html backplate-bg-image-007-ref.html
 fuzzy-if(cocoaWidget,255-255,20-20) == backplate-bg-image-008.html backplate-bg-image-008-ref.html
 fuzzy-if(cocoaWidget,255-255,20-20) == backplate-bg-image-009.html backplate-bg-image-009-ref.html
 fuzzy-if(cocoaWidget,255-255,1495-1495) fuzzy-if(winWidget,255-255,353-353) fuzzy-if(Android,255-255,700-700) == backplate-bg-image-010.html backplate-bg-image-010-ref.html
 fuzzy-if(cocoaWidget,255-255,284-320) == backplate-bg-image-011.html backplate-bg-image-011-ref.html
 fuzzy-if(cocoaWidget,255-255,16-16) == backplate-bg-image-012.html backplate-bg-image-012-ref.html