Bug 1384542: Update reftest expectations for bogus first-line support in stylo. r?bz draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 26 Jul 2017 16:45:06 +0200
changeset 616834 fae27b99bef4b637cefcc9e3267205af7fe226cf
parent 616833 5367d8e57189ed6d86ee4725da7c06921e51342b
child 616835 73e573c59e3ca7d1008391423d6a2abfd915a301
push id70824
push userbmo:emilio+bugs@crisal.io
push dateThu, 27 Jul 2017 14:50:07 +0000
reviewersbz
bugs1384542
milestone56.0a1
Bug 1384542: Update reftest expectations for bogus first-line support in stylo. r?bz MozReview-Commit-ID: AjZhnHlzON6
layout/reftests/bugs/reftest.list
layout/style/GeckoStyleContext.cpp
layout/style/nsRuleNode.cpp
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
servo/components/style/style_adjuster.rs
servo/components/style/values/computed/mod.rs
--- a/layout/reftests/bugs/reftest.list
+++ b/layout/reftests/bugs/reftest.list
@@ -1378,17 +1378,17 @@ pref(browser.display.focus_ring_width,1)
 == 495385-2d.html 495385-2-ref.html
 == 495385-2e.html 495385-2-ref.html
 pref(dom.use_xbl_scopes_for_remote_xul,true) == 495385-2f.xhtml 495385-2-ref.html
 == 495385-2g.html 495385-2-ref.html
 == 495385-2h.html 495385-2-ref.html
 == 495385-2i.html 495385-2-ref.html
 == 495385-3.html 495385-3-ref.html
 == 495385-4.html 495385-4-ref.html
-fails-if(styloVsGecko) == 495385-5.html 495385-5-ref.html
+fails-if(styloVsGecko||stylo) == 495385-5.html 495385-5-ref.html
 fails-if(styloVsGecko||stylo) == 496032-1.html 496032-1-ref.html
 == 496840-1.html 496840-1-ref.html
 fuzzy-if(skiaContent,1,17000) == 498228-1.xul 498228-1-ref.xul
 == 501037.html 501037-ref.html
 == 501257-1a.html 501257-1-ref.html
 == 501257-1b.html 501257-1-ref.html
 == 501257-1.xhtml 501257-1-ref.xhtml
 == 501627-1.html 501627-1-ref.html
--- a/layout/style/GeckoStyleContext.cpp
+++ b/layout/style/GeckoStyleContext.cpp
@@ -392,16 +392,17 @@ GeckoStyleContext::GetUniqueStyleData(co
 #define UNIQUE_CASE(c_)                                                       \
   case eStyleStruct_##c_:                                                     \
     result = new (presContext) nsStyle##c_(                                   \
       * static_cast<const nsStyle##c_ *>(current));                           \
     break;
 
   UNIQUE_CASE(Font)
   UNIQUE_CASE(Display)
+  UNIQUE_CASE(Position)
   UNIQUE_CASE(Text)
   UNIQUE_CASE(TextReset)
   UNIQUE_CASE(Visibility)
 
 #undef UNIQUE_CASE
 
   default:
     NS_ERROR("Struct type not supported.  Please find another way to do this if you can!");
@@ -860,16 +861,26 @@ GeckoStyleContext::ApplyStyleFixups(bool
         text->mTextAlign == NS_STYLE_TEXT_ALIGN_MOZ_CENTER ||
         text->mTextAlign == NS_STYLE_TEXT_ALIGN_MOZ_RIGHT)
     {
       nsStyleText* uniqueText = GET_UNIQUE_STYLE_DATA(Text);
       uniqueText->mTextAlign = NS_STYLE_TEXT_ALIGN_START;
     }
   }
 
+  // Fixup the justify-items: auto value based on our parent style here if
+  // needed.
+  if (mParent &&
+      mParent->StylePosition()->mJustifyItems & NS_STYLE_JUSTIFY_LEGACY &&
+      StylePosition()->mSpecifiedJustifyItems == NS_STYLE_JUSTIFY_AUTO &&
+      StylePosition()->mJustifyItems != mParent->StylePosition()->mJustifyItems) {
+    nsStylePosition* uniquePosition = GET_UNIQUE_STYLE_DATA(Position);
+    uniquePosition->mJustifyItems = mParent->StylePosition()->mJustifyItems;
+  }
+
   // CSS2.1 section 9.2.4 specifies fixups for the 'display' property of
   // the root element.  We can't implement them in nsRuleNode because we
   // don't want to store all display structs that aren't 'block',
   // 'inline', or 'table' in the style context tree on the off chance
   // that the root element has its style reresolved later.  So do them
   // here if needed, by changing the style data, so that other code
   // doesn't get confused by looking at the style data.
   if (!mParent &&
--- a/layout/style/nsRuleNode.cpp
+++ b/layout/style/nsRuleNode.cpp
@@ -8510,31 +8510,16 @@ SetGridLine(const nsCSSValue& aValue,
       }
       item = item->mNext;
     } while (item);
     MOZ_ASSERT(!aResult.IsAuto(),
                "should have set something away from default value");
   }
 }
 
-static uint8_t
-ComputedJustifyItems(uint8_t aSpecifiedJustifiedItems,
-                     uint8_t aParentJustifyItems)
-{
-  if (aSpecifiedJustifiedItems != NS_STYLE_JUSTIFY_AUTO) {
-    return aSpecifiedJustifiedItems;
-  }
-
-  if (aParentJustifyItems & NS_STYLE_JUSTIFY_LEGACY) {
-    return aParentJustifyItems;
-  }
-
-  return NS_STYLE_JUSTIFY_NORMAL;
-}
-
 const void*
 nsRuleNode::ComputePositionData(void* aStartStruct,
                                 const nsRuleData* aRuleData,
                                 GeckoStyleContext* aContext,
                                 nsRuleNode* aHighestNode,
                                 const RuleDetail aRuleDetail,
                                 const RuleNodeCacheConditions aConditions)
 {
@@ -8680,25 +8665,24 @@ nsRuleNode::ComputePositionData(void* aS
         : NS_STYLE_JUSTIFY_NORMAL;
     conditions.SetUncacheable();
   } else {
     SetValue(justifyItemsValue,
              pos->mSpecifiedJustifyItems, conditions,
              SETVAL_ENUMERATED | SETVAL_UNSET_INITIAL,
              parentPos->mSpecifiedJustifyItems, // unused, we handle 'inherit' above
              NS_STYLE_JUSTIFY_AUTO);
-    if (pos->mSpecifiedJustifyItems == NS_STYLE_JUSTIFY_AUTO) {
-      // FIXME(emilio): This is kind of unfortunate because this is a reset
-      // property and AUTO is the default value... Can we do better?
-      conditions.SetUncacheable();
-    }
-  }
-
-  pos->mJustifyItems = ComputedJustifyItems(pos->mSpecifiedJustifyItems,
-                                            parentPos->mJustifyItems);
+  }
+
+  // NOTE(emilio): Even though the AUTO value may depend on the parent style
+  // context, we handle that uncommon special case in ApplyStyleFixups,
+  // instead of disabling all caching of all Position structs.
+  pos->mJustifyItems =
+    pos->mSpecifiedJustifyItems == NS_STYLE_JUSTIFY_AUTO
+      ? NS_STYLE_JUSTIFY_NORMAL : pos->mSpecifiedJustifyItems;
 
   // justify-self: enum, inherit, initial
   SetValue(*aRuleData->ValueForJustifySelf(),
            pos->mJustifySelf, conditions,
            SETVAL_ENUMERATED | SETVAL_UNSET_INITIAL,
            parentPos->mJustifySelf,
            NS_STYLE_JUSTIFY_AUTO);
 
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -55,17 +55,17 @@ use properties::animated_properties::Tra
 use properties::computed_value_flags::ComputedValueFlags;
 use properties::{longhands, FontComputationData, Importance, LonghandId};
 use properties::{PropertyDeclaration, PropertyDeclarationBlock, PropertyDeclarationId};
 use rule_tree::StrongRuleNode;
 use selector_parser::PseudoElement;
 use servo_arc::{Arc, RawOffsetArc};
 use std::mem::{forget, uninitialized, transmute, zeroed};
 use std::{cmp, ops, ptr};
-use values::{Auto, CustomIdent, Either, KeyframesName};
+use values::{self, Auto, CustomIdent, Either, KeyframesName};
 use values::computed::ToComputedValue;
 use values::computed::effects::{BoxShadow, Filter, SimpleShadow};
 use values::computed::length::Percentage;
 use computed_values::border_style;
 
 pub mod style_structs {
     % for style_struct in data.style_structs:
     pub use super::${style_struct.gecko_struct_name} as ${style_struct.name};
@@ -1353,24 +1353,33 @@ fn static_assert() {
 
     % for kind in ["align", "justify"]:
     ${impl_simple_type_with_conversion(kind + "_content")}
     ${impl_simple_type_with_conversion(kind + "_self")}
     % endfor
     ${impl_simple_type_with_conversion("align_items")}
 
     pub fn set_justify_items(&mut self, v: longhands::justify_items::computed_value::T) {
-        debug_assert!(v.computed.0 != ::values::specified::align::ALIGN_AUTO);
-        self.gecko.mJustifyItems = v.computed.into();
         self.gecko.mSpecifiedJustifyItems = v.specified.into();
+        self.set_computed_justify_items(v.computed);
+    }
+
+    pub fn set_computed_justify_items(&mut self, v: values::specified::JustifyItems) {
+        debug_assert!(v.0 != ::values::specified::align::ALIGN_AUTO);
+        self.gecko.mJustifyItems = v.into();
+    }
+
+    pub fn reset_justify_items(&mut self, reset_style: &Self) {
+        self.gecko.mJustifyItems = reset_style.gecko.mJustifyItems;
+        self.gecko.mSpecifiedJustifyItems = reset_style.gecko.mSpecifiedJustifyItems;
     }
 
     pub fn copy_justify_items_from(&mut self, other: &Self) {
         self.gecko.mJustifyItems = other.gecko.mJustifyItems;
-        self.gecko.mSpecifiedJustifyItems = other.gecko.mSpecifiedJustifyItems;
+        self.gecko.mSpecifiedJustifyItems = other.gecko.mJustifyItems;
     }
 
     pub fn clone_justify_items(&self) -> longhands::justify_items::computed_value::T {
         longhands::justify_items::computed_value::T {
             computed: self.gecko.mJustifyItems.into(),
             specified: self.gecko.mSpecifiedJustifyItems.into(),
         }
     }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2600,43 +2600,50 @@ impl<'a> StyleBuilder<'a> {
         }
     }
 
     % for property in data.longhands:
     % if property.ident != "font_size":
     /// Inherit `${property.ident}` from our parent style.
     #[allow(non_snake_case)]
     pub fn inherit_${property.ident}(&mut self) {
+        let inherited_struct =
         % if property.style_struct.inherited:
-        let inherited_struct =
             self.inherited_style.get_${property.style_struct.name_lower}();
         % else:
-        let inherited_struct =
             self.inherited_style_ignoring_first_line.get_${property.style_struct.name_lower}();
         % endif
+
         self.${property.style_struct.ident}.mutate()
             .copy_${property.ident}_from(
                 inherited_struct,
                 % if property.logical:
                 self.writing_mode,
                 % endif
             );
     }
 
     /// Reset `${property.ident}` to the initial value.
     #[allow(non_snake_case)]
     pub fn reset_${property.ident}(&mut self) {
-        let reset_struct = self.reset_style.get_${property.style_struct.name_lower}();
-        self.${property.style_struct.ident}.mutate()
-            .copy_${property.ident}_from(
-                reset_struct,
-                % if property.logical:
-                self.writing_mode,
-                % endif
-            );
+        let reset_struct =
+            self.reset_style.get_${property.style_struct.name_lower}();
+        % if property.ident == "justify_items":
+            // TODO(emilio): Generalise this!
+            self.${property.style_struct.ident}.mutate()
+                .reset_${property.ident}(reset_struct)
+        % else:
+            self.${property.style_struct.ident}.mutate()
+                .copy_${property.ident}_from(
+                    reset_struct,
+                    % if property.logical:
+                    self.writing_mode,
+                    % endif
+                );
+        % endif
     }
 
     % if not property.is_vector:
     /// Set the `${property.ident}` to the computed value `value`.
     #[allow(non_snake_case)]
     pub fn set_${property.ident}(
         &mut self,
         value: longhands::${property.ident}::computed_value::T
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -449,16 +449,44 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.style.inherited_flags().contains(IS_RELEVANT_LINK_VISITED)
         };
 
         if relevant_link_visited {
             self.style.flags.insert(IS_RELEVANT_LINK_VISITED);
         }
     }
 
+    /// Resolves justify-items: auto based on the inherited style if needed to
+    /// comply with:
+    ///
+    /// https://drafts.csswg.org/css-align/#valdef-justify-items-auto
+    #[cfg(feature = "gecko")]
+    fn adjust_for_justify_items(&mut self) {
+        use values::specified::align;
+        let justify_items = self.style.get_position().clone_justify_items();
+        if justify_items.specified.0 != align::ALIGN_AUTO {
+            return;
+        }
+
+        let parent_justify_items =
+            self.style.get_parent_position().clone_justify_items();
+
+        if !parent_justify_items.computed.0.contains(align::ALIGN_LEGACY) {
+            return;
+        }
+
+        if parent_justify_items.computed == justify_items.computed {
+            return;
+        }
+
+        self.style
+            .mutate_position()
+            .set_computed_justify_items(parent_justify_items.computed);
+    }
+
     /// Adjusts the style to account for various fixups that don't fit naturally
     /// into the cascade.
     ///
     /// When comparing to Gecko, this is similar to the work done by
     /// `nsStyleContext::ApplyStyleFixups`, plus some parts of
     /// `nsStyleSet::GetContext`.
     pub fn adjust(&mut self,
                   layout_parent_style: &ComputedValues,
@@ -478,16 +506,17 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             self.adjust_for_table_text_align();
             self.adjust_for_contain();
             self.adjust_for_mathvariant();
         }
         #[cfg(feature = "servo")]
         {
             self.adjust_for_alignment(layout_parent_style);
         }
+        self.adjust_for_justify_items();
         self.adjust_for_border_width();
         self.adjust_for_outline();
         self.adjust_for_writing_mode(layout_parent_style);
         self.adjust_for_text_decoration_lines(layout_parent_style);
         #[cfg(feature = "gecko")]
         {
             self.adjust_for_ruby(layout_parent_style, flags);
         }
--- a/servo/components/style/values/computed/mod.rs
+++ b/servo/components/style/values/computed/mod.rs
@@ -429,17 +429,17 @@ impl JustifyItems {
     }
 }
 
 #[cfg(feature = "gecko")]
 impl ToComputedValue for specified::JustifyItems {
     type ComputedValue = JustifyItems;
 
     // https://drafts.csswg.org/css-align/#valdef-justify-items-auto
-    fn to_computed_value(&self, context: &Context) -> JustifyItems {
+    fn to_computed_value(&self, _context: &Context) -> JustifyItems {
         use values::specified::align;
         let specified = *self;
         let computed =
             if self.0 != align::ALIGN_AUTO {
                 *self
             } else {
                 // If the inherited value of `justify-items` includes the
                 // `legacy` keyword, `auto` computes to the inherited value,