Bug 1353164 Part 2 - Change dominant-baseline from reset to inherit r…

…=heycam
iaxat · Jul 19, 2019 · 1f1135c · 1f1135c
1 parent c2da616
commit 1f1135c
Show file tree

Hide file tree

Showing 8 changed files with 27 additions and 41 deletions.
diff --git a/layout/generic/nsFrame.cpp b/layout/generic/nsFrame.cpp
@@ -9935,14 +9935,7 @@ static StyleVerticalAlignKeyword ConvertSVGDominantBaselineToVerticalAlign(
 
 Maybe<StyleVerticalAlignKeyword> nsIFrame::VerticalAlignEnum() const {
   if (nsSVGUtils::IsInSVGTextSubtree(this)) {
-    uint8_t dominantBaseline;
-    for (const nsIFrame* frame = this; frame; frame = frame->GetParent()) {
-      dominantBaseline = frame->StyleSVGReset()->mDominantBaseline;
-      if (dominantBaseline != NS_STYLE_DOMINANT_BASELINE_AUTO ||
-          frame->IsSVGTextFrame()) {
-        break;
-      }
-    }
+    uint8_t dominantBaseline = StyleSVG()->mDominantBaseline;
     return Some(ConvertSVGDominantBaselineToVerticalAlign(dominantBaseline));
   }
 

diff --git a/layout/style/nsStyleStruct.cpp b/layout/style/nsStyleStruct.cpp
@@ -754,6 +754,7 @@ nsStyleSVG::nsStyleSVG(const Document& aDocument)
       mShapeRendering(NS_STYLE_SHAPE_RENDERING_AUTO),
       mStrokeLinecap(NS_STYLE_STROKE_LINECAP_BUTT),
       mStrokeLinejoin(NS_STYLE_STROKE_LINEJOIN_MITER),
+      mDominantBaseline(NS_STYLE_DOMINANT_BASELINE_AUTO),
       mTextAnchor(NS_STYLE_TEXT_ANCHOR_START),
       mContextFlags(
           (eStyleSVGOpacitySource_Normal << FILL_OPACITY_SOURCE_SHIFT) |
@@ -784,6 +785,7 @@ nsStyleSVG::nsStyleSVG(const nsStyleSVG& aSource)
       mShapeRendering(aSource.mShapeRendering),
       mStrokeLinecap(aSource.mStrokeLinecap),
       mStrokeLinejoin(aSource.mStrokeLinejoin),
+      mDominantBaseline(aSource.mDominantBaseline),
       mTextAnchor(aSource.mTextAnchor),
       mContextFlags(aSource.mContextFlags) {
   MOZ_COUNT_CTOR(nsStyleSVG);
@@ -836,11 +838,13 @@ nsChangeHint nsStyleSVG::CalcDifference(const nsStyleSVG& aNewData) const {
   // we need a reflow here. No intrinsic sizes need to change, so
   // nsChangeHint_NeedReflow is sufficient.
   // Note that stroke-dashoffset does not affect SVGGeometryFrame::mRect.
-  // text-anchor changes also require a reflow since it changes frames' rects.
+  // text-anchor and dominant-baseline changes also require a reflow since
+  // they change frames' rects.
   if (mStrokeWidth != aNewData.mStrokeWidth ||
       mStrokeMiterlimit != aNewData.mStrokeMiterlimit ||
       mStrokeLinecap != aNewData.mStrokeLinecap ||
       mStrokeLinejoin != aNewData.mStrokeLinejoin ||
+      mDominantBaseline != aNewData.mDominantBaseline ||
       mTextAnchor != aNewData.mTextAnchor) {
     return hint | nsChangeHint_NeedReflow |
            nsChangeHint_NeedDirtyReflow |  // XXX remove me: bug 876085
@@ -1035,7 +1039,6 @@ nsStyleSVGReset::nsStyleSVGReset(const Document& aDocument)
       mLightingColor(StyleColor::White()),
       mStopOpacity(1.0f),
       mFloodOpacity(1.0f),
-      mDominantBaseline(NS_STYLE_DOMINANT_BASELINE_AUTO),
       mVectorEffect(NS_STYLE_VECTOR_EFFECT_NONE),
       mMaskType(NS_STYLE_MASK_TYPE_LUMINANCE) {
   MOZ_COUNT_CTOR(nsStyleSVGReset);
@@ -1058,7 +1061,6 @@ nsStyleSVGReset::nsStyleSVGReset(const nsStyleSVGReset& aSource)
       mLightingColor(aSource.mLightingColor),
       mStopOpacity(aSource.mStopOpacity),
       mFloodOpacity(aSource.mFloodOpacity),
-      mDominantBaseline(aSource.mDominantBaseline),
       mVectorEffect(aSource.mVectorEffect),
       mMaskType(aSource.mMaskType) {
   MOZ_COUNT_CTOR(nsStyleSVGReset);
@@ -1114,10 +1116,7 @@ nsChangeHint nsStyleSVGReset::CalcDifference(
     hint |= nsChangeHint_UpdateEffects | nsChangeHint_RepaintFrame;
   }
 
-  if (mDominantBaseline != aNewData.mDominantBaseline) {
-    // XXXjwatt: why NS_STYLE_HINT_REFLOW? Isn't that excessive?
-    hint |= NS_STYLE_HINT_REFLOW;
-  } else if (mVectorEffect != aNewData.mVectorEffect) {
+  if (mVectorEffect != aNewData.mVectorEffect) {
     // Stroke currently affects SVGGeometryFrame::mRect, and
     // vector-effect affect stroke. As a result we need to reflow if
     // vector-effect changes in order to have SVGGeometryFrame::

diff --git a/layout/style/nsStyleStruct.h b/layout/style/nsStyleStruct.h
@@ -2303,11 +2303,12 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleSVG {
   uint8_t mColorInterpolation;         // NS_STYLE_COLOR_INTERPOLATION_*
   uint8_t mColorInterpolationFilters;  // NS_STYLE_COLOR_INTERPOLATION_*
   mozilla::StyleFillRule mFillRule;
-  uint8_t mPaintOrder;      // bitfield of NS_STYLE_PAINT_ORDER_* values
-  uint8_t mShapeRendering;  // NS_STYLE_SHAPE_RENDERING_*
-  uint8_t mStrokeLinecap;   // NS_STYLE_STROKE_LINECAP_*
-  uint8_t mStrokeLinejoin;  // NS_STYLE_STROKE_LINEJOIN_*
-  uint8_t mTextAnchor;      // NS_STYLE_TEXT_ANCHOR_*
+  uint8_t mPaintOrder;        // bitfield of NS_STYLE_PAINT_ORDER_* values
+  uint8_t mShapeRendering;    // NS_STYLE_SHAPE_RENDERING_*
+  uint8_t mStrokeLinecap;     // NS_STYLE_STROKE_LINECAP_*
+  uint8_t mStrokeLinejoin;    // NS_STYLE_STROKE_LINEJOIN_*
+  uint8_t mDominantBaseline;  // NS_STYLE_DOMINANT_BASELINE_*
+  uint8_t mTextAnchor;        // NS_STYLE_TEXT_ANCHOR_*
 
   /// Returns true if style has been set to expose the computed values of
   /// certain properties (such as 'fill') to the contents of any linked images.
@@ -2415,9 +2416,8 @@ struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleSVGReset {
   float mStopOpacity;
   float mFloodOpacity;
 
-  uint8_t mDominantBaseline;  // NS_STYLE_DOMINANT_BASELINE_*
-  uint8_t mVectorEffect;      // NS_STYLE_VECTOR_EFFECT_*
-  uint8_t mMaskType;          // NS_STYLE_MASK_TYPE_*
+  uint8_t mVectorEffect;  // NS_STYLE_VECTOR_EFFECT_*
+  uint8_t mMaskType;      // NS_STYLE_MASK_TYPE_*
 };
 
 struct MOZ_NEEDS_MEMMOVABLE_MEMBERS nsStyleEffects {

diff --git a/layout/style/test/property_database.js b/layout/style/test/property_database.js
@@ -8153,7 +8153,7 @@ var gCSSProperties = {
   },
   "dominant-baseline": {
     domProp: "dominantBaseline",
-    inherited: false,
+    inherited: true,
     type: CSS_TYPE_LONGHAND,
     initial_values: ["auto"],
     other_values: [

diff --git a/layout/svg/SVGTextFrame.cpp b/layout/svg/SVGTextFrame.cpp
@@ -1520,7 +1520,7 @@ class TextFrameIterator {
       return;
     }
 
-    mBaselines.AppendElement(mRootFrame->StyleSVGReset()->mDominantBaseline);
+    mBaselines.AppendElement(mRootFrame->StyleSVG()->mDominantBaseline);
     Next();
   }
 
@@ -1663,10 +1663,7 @@ nsTextFrame* TextFrameIterator::Next() {
 }
 
 void TextFrameIterator::PushBaseline(nsIFrame* aNextFrame) {
-  uint8_t baseline = aNextFrame->StyleSVGReset()->mDominantBaseline;
-  if (baseline == NS_STYLE_DOMINANT_BASELINE_AUTO) {
-    baseline = mBaselines.LastElement();
-  }
+  uint8_t baseline = aNextFrame->StyleSVG()->mDominantBaseline;
   mBaselines.AppendElement(baseline);
 }
 

diff --git a/servo/components/style/properties/longhands/inherited_svg.mako.rs b/servo/components/style/properties/longhands/inherited_svg.mako.rs
@@ -10,6 +10,15 @@
 
 // Section 10 - Text
 
+${helpers.single_keyword(
+    "dominant-baseline",
+    """auto ideographic alphabetic hanging mathematical central middle
+       text-after-edge text-before-edge""",
+    products="gecko",
+    animation_value_type="discrete",
+    spec="https://www.w3.org/TR/css-inline-3/#propdef-dominant-baseline",
+)}
+
 ${helpers.single_keyword(
     "text-anchor",
     "start middle end",

diff --git a/servo/components/style/properties/longhands/svg.mako.rs b/servo/components/style/properties/longhands/svg.mako.rs
@@ -6,15 +6,6 @@
 
 <% data.new_style_struct("SVG", inherited=False, gecko_name="SVGReset") %>
 
-${helpers.single_keyword(
-    "dominant-baseline",
-    """auto ideographic alphabetic hanging mathematical central middle
-       text-after-edge text-before-edge""",
-    products="gecko",
-    animation_value_type="discrete",
-    spec="https://www.w3.org/TR/SVG11/text.html#DominantBaselineProperty",
-)}
-
 ${helpers.single_keyword(
     "vector-effect",
     "none non-scaling-stroke",

diff --git a/testing/web-platform/meta/css/css-inline/inheritance.html.ini b/testing/web-platform/meta/css/css-inline/inheritance.html.ini
@@ -1,7 +1,4 @@
 [inheritance.html]
-  [Property dominant-baseline inherits]
-    expected: FAIL
-
   [Property dominant-baseline has initial value normal]
     expected: FAIL