Bug 1383868 - Compute display of <legend> other than 'none' to 'block'. draft
authorXidorn Quan <me@upsuper.org>
Fri, 15 Sep 2017 10:01:36 +1000
changeset 665174 50ac752dbd97992c2d52b0c69eb4fc95f87baadc
parent 665168 c8d9125a098e4df5ea3bd60462163f9994b1fdb6
child 731671 c6d29fe45e4f91dee7a8cca4b394d15178e0aae1
push id79948
push userxquan@mozilla.com
push dateFri, 15 Sep 2017 00:24:47 +0000
bugs1383868
milestone57.0a1
Bug 1383868 - Compute display of <legend> other than 'none' to 'block'. MozReview-Commit-ID: GgafVCoDhFL
servo/components/style/properties/properties.mako.rs
servo/components/style/style_adjuster.rs
servo/components/style/style_resolver.rs
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -3034,17 +3034,17 @@ pub type CascadePropertyFn =
 static CASCADE_PROPERTY: [CascadePropertyFn; ${len(data.longhands)}] = [
     % for property in data.longhands:
         longhands::${property.ident}::cascade_property,
     % endfor
 ];
 
 bitflags! {
     /// A set of flags to tweak the behavior of the `cascade` function.
-    pub flags CascadeFlags: u8 {
+    pub flags CascadeFlags: u16 {
         /// Whether to inherit all styles from the parent. If this flag is not
         /// present, non-inherited styles are reset to their initial values.
         const INHERIT_ALL = 1,
 
         /// Whether to skip any display style fixup for root element, flex/grid
         /// item, and ruby descendants.
         const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 1 << 1,
 
@@ -3071,16 +3071,19 @@ bitflags! {
 
         /// Whether we're computing the style of a link, either visited or
         /// unvisited.
         const IS_LINK = 1 << 6,
 
         /// Whether we're computing the style of a link element that happens to
         /// be visited.
         const IS_VISITED_LINK = 1 << 7,
+
+        /// Whether we're computing the style of a legend element.
+        const IS_LEGEND = 1 << 8,
     }
 }
 
 /// Performs the CSS cascade, computing new styles for an element from its parent style.
 ///
 /// The arguments are:
 ///
 ///   * `device`: Used to get the initial viewport and other external state.
--- a/servo/components/style/style_adjuster.rs
+++ b/servo/components/style/style_adjuster.rs
@@ -361,16 +361,38 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
             display::inline_grid => Some(display::grid),
             _ => None,
         };
         if let Some(new_display) = new_display {
             self.style.mutate_box().set_display(new_display);
         }
     }
 
+    /// Convert display value of <legend> other than `none` to block.
+    ///
+    /// Note that, this needs to happen before any inlinification like
+    /// adjust_for_ruby, otherwise we may break their expectations.
+    ///
+    /// See https://github.com/whatwg/html/pull/3042
+    ///
+    /// Also note that we may actually want to include `contents` here
+    /// as well, since it doesn't generate boxes just like `none`. But
+    /// since other browsers don't do that, we should probably try to
+    /// change it later.
+    fn adjust_for_legend(&mut self, flags: CascadeFlags) {
+        use properties::IS_LEGEND;
+        if !flags.contains(IS_LEGEND) {
+            return;
+        }
+        match self.style.get_box().clone_display() {
+            display::none | display::block => {},
+            _ => self.style.mutate_box().set_display(display::block),
+        }
+    }
+
     /// -moz-center, -moz-left and -moz-right are used for HTML's alignment.
     ///
     /// This is covering the <div align="right"><table>...</table></div> case.
     ///
     /// In this case, we don't want to inherit the text alignment into the
     /// table.
     #[cfg(feature = "gecko")]
     fn adjust_for_table_text_align(&mut self) {
@@ -558,16 +580,17 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
         flags: CascadeFlags,
     ) {
         self.adjust_for_visited(flags);
         #[cfg(feature = "gecko")]
         {
             self.adjust_for_prohibited_display_contents(flags);
             self.adjust_for_fieldset_content(layout_parent_style, flags);
         }
+        self.adjust_for_legend(flags);
         self.adjust_for_top_layer();
         self.blockify_if_necessary(layout_parent_style, flags);
         self.adjust_for_position();
         self.adjust_for_overflow();
         #[cfg(feature = "gecko")]
         {
             self.adjust_for_table_text_align();
             self.adjust_for_contain();
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -6,17 +6,17 @@
 
 use applicable_declarations::ApplicableDeclarationList;
 use context::{CascadeInputs, ElementCascadeInputs, StyleContext};
 use data::{ElementStyles, EagerPseudoStyles};
 use dom::TElement;
 use log::LogLevel::Trace;
 use matching::{CascadeVisitedMode, MatchMethods};
 use properties::{AnimationRules, CascadeFlags, ComputedValues};
-use properties::{IS_LINK, IS_ROOT_ELEMENT, IS_VISITED_LINK};
+use properties::{IS_LEGEND, IS_LINK, IS_ROOT_ELEMENT, IS_VISITED_LINK};
 use properties::{PROHIBIT_DISPLAY_CONTENTS, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP};
 use properties::{VISITED_DEPENDENT_ONLY, cascade};
 use properties::longhands::display::computed_value::T as display;
 use rule_tree::StrongRuleNode;
 use selector_parser::{PseudoElement, SelectorImpl};
 use selectors::matching::{ElementSelectorFlags, MatchingContext, MatchingMode, VisitedHandlingMode};
 use servo_arc::Arc;
 use stylist::RuleInclusion;
@@ -543,21 +543,25 @@ where
 
         let mut cascade_flags = CascadeFlags::empty();
 
         if self.element.skip_root_and_item_based_display_fixup() ||
            pseudo.map_or(false, |p| p.skip_item_based_display_fixup()) {
             cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP);
         }
 
-        if pseudo.is_none() && self.element.is_link() {
-            cascade_flags.insert(IS_LINK);
-            if self.element.is_visited_link() &&
-                self.context.shared.visited_styles_enabled {
-                cascade_flags.insert(IS_VISITED_LINK);
+        if pseudo.is_none() {
+            if self.element.is_link() {
+                cascade_flags.insert(IS_LINK);
+                if self.element.is_visited_link() &&
+                    self.context.shared.visited_styles_enabled {
+                    cascade_flags.insert(IS_VISITED_LINK);
+                }
+            } else if self.element.get_local_name() == &*local_name!("legend") {
+                cascade_flags.insert(IS_LEGEND);
             }
         }
 
         if cascade_visited.visited_dependent_only() {
             // If this element is a link, we want its visited style to inherit
             // from the regular style of its parent, because only the
             // visitedness of the relevant link should influence style.
             if pseudo.is_some() || !self.element.is_link() {