Bug 1266621 part 2 - Convert column-rule-color to store complex color. r=heycam draft
authorXidorn Quan <me@upsuper.org>
Tue, 27 Sep 2016 20:12:08 +1000
changeset 418915 99766bfc2f7558935fa5adc869fa23844e3b909f
parent 418914 6c68c6e26648354d4d5515e199fdf07711eca726
child 418916 18ff94f359dde9cd174e2991b7dd848686058057
push id30801
push userxquan@mozilla.com
push dateThu, 29 Sep 2016 09:22:37 +0000
reviewersheycam
bugs1266621
milestone52.0a1
Bug 1266621 part 2 - Convert column-rule-color to store complex color. r=heycam MozReview-Commit-ID: AB2b5CPABOZ
layout/style/StyleAnimationValue.cpp
layout/style/nsCSSPropList.h
layout/style/nsComputedDOMStyle.cpp
layout/style/nsRuleNode.cpp
layout/style/nsStyleContext.cpp
layout/style/nsStyleStruct.cpp
layout/style/nsStyleStruct.h
layout/style/test/test_transitions_events.html
layout/style/test/test_transitions_per_property.html
--- a/layout/style/StyleAnimationValue.cpp
+++ b/layout/style/StyleAnimationValue.cpp
@@ -3857,29 +3857,16 @@ StyleAnimationValue::ExtractComputedValu
             static_cast<const nsStyleOutline*>(styleStruct);
           nscolor color;
           if (!styleOutline->GetOutlineColor(color))
             color = aStyleContext->StyleColor()->mColor;
           aComputedValue.SetColorValue(color);
           break;
         }
 
-        case eCSSProperty_column_rule_color: {
-          const nsStyleColumn *styleColumn =
-            static_cast<const nsStyleColumn*>(styleStruct);
-          nscolor color;
-          if (styleColumn->mColumnRuleColorIsForeground) {
-            color = aStyleContext->StyleColor()->mColor;
-          } else {
-            color = styleColumn->mColumnRuleColor;
-          }
-          aComputedValue.SetColorValue(color);
-          break;
-        }
-
         case eCSSProperty_column_count: {
           const nsStyleColumn *styleColumn =
             static_cast<const nsStyleColumn*>(styleStruct);
           if (styleColumn->mColumnCount == NS_STYLE_COLUMN_COUNT_AUTO) {
             aComputedValue.SetAutoValue();
           } else {
             aComputedValue.SetIntValue(styleColumn->mColumnCount,
                                        eUnit_Integer);
--- a/layout/style/nsCSSPropList.h
+++ b/layout/style/nsCSSPropList.h
@@ -1517,18 +1517,18 @@ CSS_PROP_COLUMN(
     -moz-column-rule-color,
     column_rule_color,
     CSS_PROP_DOMPROP_PREFIXED(ColumnRuleColor),
     CSS_PROPERTY_PARSE_VALUE |
         CSS_PROPERTY_IGNORED_WHEN_COLORS_DISABLED,
     "",
     VARIANT_HCK,
     kBorderColorKTable,
-    CSS_PROP_NO_OFFSET,
-    eStyleAnimType_Custom)
+    offsetof(nsStyleColumn, mColumnRuleColor),
+    eStyleAnimType_ComplexColor)
 CSS_PROP_COLUMN(
     -moz-column-rule-style,
     column_rule_style,
     CSS_PROP_DOMPROP_PREFIXED(ColumnRuleStyle),
     CSS_PROPERTY_PARSE_VALUE,
     "",
     VARIANT_HK,
     kBorderStyleKTable,
--- a/layout/style/nsComputedDOMStyle.cpp
+++ b/layout/style/nsComputedDOMStyle.cpp
@@ -1059,26 +1059,17 @@ nsComputedDOMStyle::DoGetColumnRuleStyle
                                    nsCSSProps::kBorderStyleKTable));
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetColumnRuleColor()
 {
   RefPtr<nsROCSSPrimitiveValue> val = new nsROCSSPrimitiveValue;
-
-  const nsStyleColumn* column = StyleColumn();
-  nscolor ruleColor;
-  if (column->mColumnRuleColorIsForeground) {
-    ruleColor = StyleColor()->mColor;
-  } else {
-    ruleColor = column->mColumnRuleColor;
-  }
-
-  SetToRGBAColor(val, ruleColor);
+  SetValueFromComplexColor(val, StyleColumn()->mColumnRuleColor);
   return val.forget();
 }
 
 already_AddRefed<CSSValue>
 nsComputedDOMStyle::DoGetContent()
 {
   const nsStyleContent *content = StyleContent();
 
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -9237,40 +9237,21 @@ nsRuleNode::ComputeColumnData(void* aSta
     column->mColumnRuleStyle = NS_STYLE_BORDER_STYLE_NONE;
   }
   else if (eCSSUnit_Inherit == styleValue.GetUnit()) {
     conditions.SetUncacheable();
     column->mColumnRuleStyle = parent->mColumnRuleStyle;
   }
 
   // column-rule-color: color, inherit
-  const nsCSSValue& colorValue = *aRuleData->ValueForColumnRuleColor();
-  if (eCSSUnit_Inherit == colorValue.GetUnit()) {
-    conditions.SetUncacheable();
-    column->mColumnRuleColorIsForeground = false;
-    if (parent->mColumnRuleColorIsForeground) {
-      if (parentContext) {
-        column->mColumnRuleColor = parentContext->StyleColor()->mColor;
-      } else {
-        nsStyleColor defaultColumnRuleColor(mPresContext);
-        column->mColumnRuleColor = defaultColumnRuleColor.mColor;
-      }
-    } else {
-      column->mColumnRuleColor = parent->mColumnRuleColor;
-    }
-  }
-  else if (eCSSUnit_Initial == colorValue.GetUnit() ||
-           eCSSUnit_Unset == colorValue.GetUnit() ||
-           eCSSUnit_Enumerated == colorValue.GetUnit()) {
-    column->mColumnRuleColorIsForeground = true;
-  }
-  else if (SetColor(colorValue, 0, mPresContext, aContext,
-                    column->mColumnRuleColor, conditions)) {
-    column->mColumnRuleColorIsForeground = false;
-  }
+  SetComplexColor<eUnsetInitial>(*aRuleData->ValueForColumnRuleColor(),
+                                 parent->mColumnRuleColor,
+                                 StyleComplexColor::CurrentColor(),
+                                 mPresContext,
+                                 column->mColumnRuleColor, conditions);
 
   // column-fill: enum
   SetValue(*aRuleData->ValueForColumnFill(),
            column->mColumnFill, conditions,
            SETVAL_ENUMERATED | SETVAL_UNSET_INITIAL,
            parent->mColumnFill,
            NS_STYLE_COLUMN_FILL_BALANCE);
 
--- a/layout/style/nsStyleContext.cpp
+++ b/layout/style/nsStyleContext.cpp
@@ -1206,19 +1206,17 @@ nsStyleContext::CalcStyleDifferenceInter
         change = true;
       }
     }
 
     // NB: Calling Peek on |this|, not |thisVis| (see above).
     if (!change && PeekStyleColumn()) {
       const nsStyleColumn *thisVisColumn = thisVis->StyleColumn();
       const nsStyleColumn *otherVisColumn = otherVis->StyleColumn();
-      if (thisVisColumn->mColumnRuleColor != otherVisColumn->mColumnRuleColor ||
-          thisVisColumn->mColumnRuleColorIsForeground !=
-            otherVisColumn->mColumnRuleColorIsForeground) {
+      if (thisVisColumn->mColumnRuleColor != otherVisColumn->mColumnRuleColor) {
         change = true;
       }
     }
 
     // NB: Calling Peek on |this|, not |thisVis| (see above).
     if (!change && PeekStyleText()) {
       const nsStyleText* thisVisText = thisVis->StyleText();
       const nsStyleText* otherVisText = otherVis->StyleText();
--- a/layout/style/nsStyleStruct.cpp
+++ b/layout/style/nsStyleStruct.cpp
@@ -881,20 +881,19 @@ nsStyleXUL::CalcDifference(const nsStyle
 // nsStyleColumn
 //
 /* static */ const uint32_t nsStyleColumn::kMaxColumnCount = 1000;
 
 nsStyleColumn::nsStyleColumn(StyleStructContext aContext)
   : mColumnCount(NS_STYLE_COLUMN_COUNT_AUTO)
   , mColumnWidth(eStyleUnit_Auto)
   , mColumnGap(eStyleUnit_Normal)
-  , mColumnRuleColor(NS_RGB(0, 0, 0))
+  , mColumnRuleColor(StyleComplexColor::CurrentColor())
   , mColumnRuleStyle(NS_STYLE_BORDER_STYLE_NONE)
   , mColumnFill(NS_STYLE_COLUMN_FILL_BALANCE)
-  , mColumnRuleColorIsForeground(true)
   , mColumnRuleWidth((StaticPresData::Get()
                         ->GetBorderWidthTable())[NS_STYLE_BORDER_WIDTH_MEDIUM])
   , mTwipsPerPixel(aContext.AppUnitsPerDevPixel())
 {
   MOZ_COUNT_CTOR(nsStyleColumn);
 }
 
 nsStyleColumn::~nsStyleColumn()
@@ -904,17 +903,16 @@ nsStyleColumn::~nsStyleColumn()
 
 nsStyleColumn::nsStyleColumn(const nsStyleColumn& aSource)
   : mColumnCount(aSource.mColumnCount)
   , mColumnWidth(aSource.mColumnWidth)
   , mColumnGap(aSource.mColumnGap)
   , mColumnRuleColor(aSource.mColumnRuleColor)
   , mColumnRuleStyle(aSource.mColumnRuleStyle)
   , mColumnFill(aSource.mColumnFill)
-  , mColumnRuleColorIsForeground(aSource.mColumnRuleColorIsForeground)
   , mColumnRuleWidth(aSource.mColumnRuleWidth)
   , mTwipsPerPixel(aSource.mTwipsPerPixel)
 {
   MOZ_COUNT_CTOR(nsStyleColumn);
 }
 
 nsChangeHint
 nsStyleColumn::CalcDifference(const nsStyleColumn& aNewData) const
@@ -931,18 +929,17 @@ nsStyleColumn::CalcDifference(const nsSt
   if (mColumnWidth != aNewData.mColumnWidth ||
       mColumnGap != aNewData.mColumnGap ||
       mColumnFill != aNewData.mColumnFill) {
     return NS_STYLE_HINT_REFLOW;
   }
 
   if (GetComputedColumnRuleWidth() != aNewData.GetComputedColumnRuleWidth() ||
       mColumnRuleStyle != aNewData.mColumnRuleStyle ||
-      mColumnRuleColor != aNewData.mColumnRuleColor ||
-      mColumnRuleColorIsForeground != aNewData.mColumnRuleColorIsForeground) {
+      mColumnRuleColor != aNewData.mColumnRuleColor) {
     return NS_STYLE_HINT_VISUAL;
   }
 
   // XXX Is it right that we never check mTwipsPerPixel to return a
   // non-nsChangeHint_NeutralChange hint?
   if (mColumnRuleWidth != aNewData.mColumnRuleWidth ||
       mTwipsPerPixel != aNewData.mTwipsPerPixel) {
     return nsChangeHint_NeutralChange;
--- a/layout/style/nsStyleStruct.h
+++ b/layout/style/nsStyleStruct.h
@@ -3482,24 +3482,20 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsSt
    * nsColumnSetFrame and nsRuleNode.
    */
   static const uint32_t kMaxColumnCount;
 
   uint32_t     mColumnCount; // [reset] see nsStyleConsts.h
   nsStyleCoord mColumnWidth; // [reset] coord, auto
   nsStyleCoord mColumnGap;   // [reset] coord, normal
 
-  nscolor      mColumnRuleColor;  // [reset]
+  mozilla::StyleComplexColor mColumnRuleColor; // [reset]
   uint8_t      mColumnRuleStyle;  // [reset]
   uint8_t      mColumnFill;  // [reset] see nsStyleConsts.h
 
-  // See https://bugzilla.mozilla.org/show_bug.cgi?id=271586#c43 for why
-  // this is hard to replace with 'currentColor'.
-  bool mColumnRuleColorIsForeground;
-
   void SetColumnRuleWidth(nscoord aWidth) {
     mColumnRuleWidth = NS_ROUND_BORDER_TO_PIXELS(aWidth, mTwipsPerPixel);
   }
 
   nscoord GetComputedColumnRuleWidth() const {
     return (IsVisibleBorderStyle(mColumnRuleStyle) ? mColumnRuleWidth : 0);
   }
 
--- a/layout/style/test/test_transitions_events.html
+++ b/layout/style/test/test_transitions_events.html
@@ -66,17 +66,16 @@ function $(id) { return document.getElem
 function cs(id) { return getComputedStyle($(id), ""); }
 
 var got_one_root = false;
 var got_one_target = false;
 var got_one_target_bordertop = false;
 var got_one_target_borderright = false;
 var got_one_target_borderbottom = false;
 var got_one_target_borderleft = false;
-var got_one_target_columnrule = false;
 var got_one_target_outlinecolor = false;
 var got_two_target = false;
 var got_three_top = false;
 var got_three_right = false;
 var got_three_bottom = false;
 var got_three_left = false;
 var got_four_root = false;
 var got_body = false;
@@ -172,22 +171,16 @@ document.documentElement.addEventListene
         event.stopPropagation();
         break;
       case "border-left-color":
         ok(!got_one_target_borderleft,
            "transitionend on one on target (border-left-color)");
         got_one_target_borderleft = true;
         event.stopPropagation();
         break;
-      case "-moz-column-rule-color":
-        ok(!got_one_target_columnrule,
-           "transitionend on one on target (-moz-column-rule-color)");
-        got_one_target_columnrule = true;
-        event.stopPropagation();
-        break;
       case "outline-color":
         ok(!got_one_target_outlinecolor,
            "transitionend on one on target (outline-color)");
         got_one_target_outlinecolor = true;
         event.stopPropagation();
         break;
       default:
         ok(false, "unexpected property name " + event.propertyName +
@@ -201,17 +194,16 @@ document.documentElement.addEventListene
   }, false);
 
 started_test(); // color on #one
 started_test(); // border-top-color on #one
 started_test(); // border-right-color on #one
 started_test(); // border-right-color on #one (listener on root)
 started_test(); // border-bottom-color on #one
 started_test(); // border-left-color on #one
-started_test(); // -moz-column-rule-color on #one
 started_test(); // outline-color on #one
 $("one").style.color = "lime";
 
 
 $("two").addEventListener("transitionend",
   function(event) {
     event.stopPropagation();
 
--- a/layout/style/test/test_transitions_per_property.html
+++ b/layout/style/test/test_transitions_per_property.html
@@ -67,18 +67,17 @@ var supported_properties = {
                        test_float_aboveOne_transition,
                        test_float_zeroToOne_clamped ],
     "box-shadow": [ test_shadow_transition ],
     "-moz-column-count": [ test_pos_integer_or_auto_transition,
                            test_integer_at_least_one_clamping ],
     "-moz-column-gap": [ test_length_transition,
                          test_length_clamped ],
     "-moz-column-rule-color": [ test_color_transition,
-                                test_currentcolor_transition,
-                                test_border_color_transition ],
+                                test_true_currentcolor_transition ],
     "-moz-column-rule-width": [ test_length_transition,
                                 test_length_clamped ],
     "-moz-column-width": [ test_length_transition,
                            test_length_clamped ],
     "-moz-image-region": [ test_rect_transition ],
     "-moz-outline-radius-bottomleft": [ test_radius_transition ],
     "-moz-outline-radius-bottomright": [ test_radius_transition ],
     "-moz-outline-radius-topleft": [ test_radius_transition ],