Auto merge of #14471 - heycam:blockification, r=emilio

adjust display style fixup to handle more Gecko cases

<!-- Please describe your changes on the following line: -->

This tweaks the display property fixup we do when restyling to:

* handle the display values that Gecko supports that Servo doesn't
* blockify grid items (like we do flex items)
* skip the fixup for NAC

And while I'm in the area, this sets `nsStyleDisplay::mOriginalDisplay` too.

r? @bholley cc @SimonSapin

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [ ] These changes fix #__ (github issue number if applicable).

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/14471)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2016-12-06 20:19:21 -08:00 committed by GitHub
commit b5a96e6054
6 changed files with 77 additions and 17 deletions

View file

@ -477,6 +477,10 @@ impl<'le> TElement for ServoLayoutElement<'le> {
}) })
} }
} }
fn skip_root_and_item_based_display_fixup(&self) -> bool {
false
}
} }
impl<'le> PartialEq for ServoLayoutElement<'le> { impl<'le> PartialEq for ServoLayoutElement<'le> {

View file

@ -269,4 +269,9 @@ pub trait TElement : PartialEq + Debug + Sized + Copy + Clone + ElementExt + Pre
fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> { fn mutate_data(&self) -> Option<AtomicRefMut<ElementData>> {
self.get_data().map(|x| x.borrow_mut()) self.get_data().map(|x| x.borrow_mut())
} }
/// Whether we should skip any root- or item-based display property
/// blockification on this element. (This function exists so that Gecko
/// native anonymous content can opt out of this style fixup.)
fn skip_root_and_item_based_display_fixup(&self) -> bool;
} }

View file

@ -371,6 +371,15 @@ impl<'le> TElement for GeckoElement<'le> {
fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> { fn get_data(&self) -> Option<&AtomicRefCell<ElementData>> {
unsafe { self.0.mServoData.get().as_ref() } unsafe { self.0.mServoData.get().as_ref() }
} }
fn skip_root_and_item_based_display_fixup(&self) -> bool {
// We don't want to fix up display values of native anonymous content.
// Additionally, we want to skip root-based display fixup for document
// level native anonymous content subtree roots, since they're not
// really roots from the style fixup perspective. Checking that we
// are NAC handles both cases.
self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) != 0
}
} }
impl<'le> PartialEq for GeckoElement<'le> { impl<'le> PartialEq for GeckoElement<'le> {

View file

@ -14,7 +14,7 @@ use cascade_info::CascadeInfo;
use context::{SharedStyleContext, StyleContext}; use context::{SharedStyleContext, StyleContext};
use data::{ComputedStyle, ElementData, ElementStyles, PseudoStyles}; use data::{ComputedStyle, ElementData, ElementStyles, PseudoStyles};
use dom::{TElement, TNode, TRestyleDamage, UnsafeNode}; use dom::{TElement, TNode, TRestyleDamage, UnsafeNode};
use properties::{CascadeFlags, ComputedValues, SHAREABLE, cascade}; use properties::{CascadeFlags, ComputedValues, SHAREABLE, SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP, cascade};
use properties::longhands::display::computed_value as display; use properties::longhands::display::computed_value as display;
use rule_tree::StrongRuleNode; use rule_tree::StrongRuleNode;
use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl}; use selector_parser::{PseudoElement, RestyleDamage, SelectorImpl};
@ -405,6 +405,9 @@ trait PrivateMatchMethods: TElement {
if booleans.shareable { if booleans.shareable {
cascade_flags.insert(SHAREABLE) cascade_flags.insert(SHAREABLE)
} }
if self.skip_root_and_item_based_display_fixup() {
cascade_flags.insert(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP)
}
let this_style = match parent_style { let this_style = match parent_style {
Some(ref parent_style) => { Some(ref parent_style) => {

View file

@ -1035,7 +1035,24 @@ fn static_assert() {
"-moz-groupbox", "-moz-groupbox",
gecko_enum_prefix="StyleDisplay", gecko_enum_prefix="StyleDisplay",
gecko_strip_moz_prefix=False) %> gecko_strip_moz_prefix=False) %>
${impl_keyword('display', 'mDisplay', display_keyword, True)}
pub fn set_display(&mut self, v: longhands::display::computed_value::T) {
use properties::longhands::display::computed_value::T as Keyword;
// FIXME(bholley): Align binary representations and ditch |match| for cast + static_asserts
let result = match v {
% for value in display_keyword.values_for('gecko'):
Keyword::${to_rust_ident(value)} =>
structs::${display_keyword.gecko_constant(value)},
% endfor
};
self.gecko.mDisplay = result;
self.gecko.mOriginalDisplay = result;
}
pub fn copy_display_from(&mut self, other: &Self) {
self.gecko.mDisplay = other.gecko.mDisplay;
self.gecko.mOriginalDisplay = other.gecko.mOriginalDisplay;
}
<%call expr="impl_keyword_clone('display', 'mDisplay', display_keyword)"></%call>
// overflow-y is implemented as a newtype of overflow-x, so we need special handling. // overflow-y is implemented as a newtype of overflow-x, so we need special handling.
// We could generalize this if we run into other newtype keywords. // We could generalize this if we run into other newtype keywords.

View file

@ -1436,6 +1436,8 @@ bitflags! {
/// Whether to inherit all styles from the parent. If this flag is not present, /// Whether to inherit all styles from the parent. If this flag is not present,
/// non-inherited styles are reset to their initial values. /// non-inherited styles are reset to their initial values.
const INHERIT_ALL = 0x02, const INHERIT_ALL = 0x02,
/// Whether to skip any root element and flex/grid item display style fixup.
const SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP = 0x04,
} }
} }
@ -1630,30 +1632,50 @@ pub fn apply_declarations<'a, F, I>(viewport_size: Size2D<Au>,
longhands::position::SpecifiedValue::absolute | longhands::position::SpecifiedValue::absolute |
longhands::position::SpecifiedValue::fixed); longhands::position::SpecifiedValue::fixed);
let floated = style.get_box().clone_float() != longhands::float::SpecifiedValue::none; let floated = style.get_box().clone_float() != longhands::float::SpecifiedValue::none;
let is_flex_item = // FIXME(heycam): We should look past any display:contents ancestors to
context.inherited_style.get_box().clone_display() == computed_values::display::T::flex; // determine if we are a flex or grid item, but we don't have access to
if positioned || floated || is_root_element || is_flex_item { // grandparent or higher style here.
let is_item = matches!(context.inherited_style.get_box().clone_display(),
% if product == "gecko":
computed_values::display::T::grid |
% endif
computed_values::display::T::flex);
let (blockify_root, blockify_item) = match flags.contains(SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) {
false => (is_root_element, is_item),
true => (false, false),
};
if positioned || floated || blockify_root || blockify_item {
use computed_values::display::T; use computed_values::display::T;
let specified_display = style.get_box().clone_display(); let specified_display = style.get_box().clone_display();
let computed_display = match specified_display { let computed_display = match specified_display {
T::inline_table => { // Values that have a corresponding block-outside version.
Some(T::table) T::inline_table => Some(T::table),
} % if product == "gecko":
T::inline | T::inline_block | T::inline_flex => Some(T::flex),
T::table_row_group | T::table_column | T::inline_grid => Some(T::grid),
T::table_column_group | T::table_header_group | T::_webkit_inline_box => Some(T::_webkit_box),
T::table_footer_group | T::table_row | T::table_cell | % endif
T::table_caption => {
Some(T::block) // Special handling for contents and list-item on the root element for Gecko.
} % if product == "gecko":
_ => None T::contents | T::list_item if blockify_root => Some(T::block),
% endif
// Values that are not changed by blockification.
T::block | T::flex | T::list_item | T::table => None,
% if product == "gecko":
T::contents | T::grid | T::_webkit_box => None,
% endif
// Everything becomes block.
_ => Some(T::block),
}; };
if let Some(computed_display) = computed_display { if let Some(computed_display) = computed_display {
let box_ = style.mutate_box(); let box_ = style.mutate_box();
box_.set_display(computed_display); box_.set_display(computed_display);
% if product == "servo": % if product == "servo":
box_.set__servo_display_for_hypothetical_box(if is_root_element || is_flex_item { box_.set__servo_display_for_hypothetical_box(if blockify_root || blockify_item {
computed_display computed_display
} else { } else {
specified_display specified_display