style: Use CascadeFlags for what they're for.

Now that we have an Element around on cascade, we can stop using the cascade
flags mechanism to pass various element-related state, like "is this element the
root", or "should it use the item-based display fixup".

That fixes handwaviness in the handling of those flags from style reparenting,
and code duplication to handle tricky stuff like :visited.

There are a number of other changes that are worth noticing:

 * skip_root_and_item_based_display_fixup is renamed to skip_item_display_fixup:

   TElement::is_root() already implies being the document element, which by
   definition is not native anonymous and not a pseudo-element.

   Thus, you never get fixed-up if your NAC or a pseudo, which is what the code
   tried to avoid, so the only fixup with a point is the item one, which is
   necessary.

 * The pseudo-element probing code was refactored to return early a
   Option::<CascadeInputs>::None, which is nicer than what it was doing.

 * The visited_links_enabled check has moved to selector-matching time. The rest
   of the checks aren't based on whether the element is a link, or are properly
   guarded by parent_style.visited_style().is_some() or visited_rules.is_some().

   Thus you can transitively infer that no element will end up with a :visited
   style, not even from style reparenting.

Anyway, the underlying reason why I want the element in StyleAdjuster is because
we're going to implement an adjustment in there depending on the tag of the
element (converting display: contents to display: none depending on the tag), so
computing that information eagerly, including a hash lookup, wouldn't be nice.
This commit is contained in:
Emilio Cobos Álvarez 2018-01-22 23:53:03 +01:00
parent 104f5c2553
commit cd04664fb9
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
11 changed files with 233 additions and 298 deletions

View file

@ -6,6 +6,7 @@
//! a computed style needs in order for it to adhere to the CSS spec.
use app_units::Au;
use dom::TElement;
use properties::{self, CascadeFlags, ComputedValues, StyleBuilder};
use properties::longhands::display::computed_value::T as Display;
use properties::longhands::float::computed_value::T as Float;
@ -50,13 +51,30 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
}
}
/// Whether we should skip any item-based display property blockification on
/// this element.
fn skip_item_display_fixup<E>(&self, element: Option<E>) -> bool
where
E: TElement,
{
if let Some(pseudo) = self.style.pseudo {
return pseudo.skip_item_display_fixup();
}
element.map_or(false, |e| e.skip_item_display_fixup())
}
/// Apply the blockification rules based on the table in CSS 2.2 section 9.7.
/// <https://drafts.csswg.org/css2/visuren.html#dis-pos-flo>
fn blockify_if_necessary(
fn blockify_if_necessary<E>(
&mut self,
layout_parent_style: &ComputedValues,
flags: CascadeFlags,
) {
element: Option<E>,
)
where
E: TElement,
{
let mut blockify = false;
macro_rules! blockify_if {
($if_what:expr) => {
@ -66,8 +84,9 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
}
}
if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) {
blockify_if!(flags.contains(CascadeFlags::IS_ROOT_ELEMENT));
let is_root = self.style.pseudo.is_none() && element.map_or(false, |e| e.is_root());
blockify_if!(is_root);
if !self.skip_item_display_fixup(element) {
blockify_if!(layout_parent_style.get_box().clone_display().is_item_container());
}
@ -81,8 +100,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
}
let display = self.style.get_box().clone_display();
let blockified_display =
display.equivalent_block_display(flags.contains(CascadeFlags::IS_ROOT_ELEMENT));
let blockified_display = display.equivalent_block_display(is_root);
if display != blockified_display {
self.style.mutate_box().set_adjusted_display(
blockified_display,
@ -477,12 +495,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
/// * suppress border and padding for ruby level containers,
/// * correct unicode-bidi.
#[cfg(feature = "gecko")]
fn adjust_for_ruby(
fn adjust_for_ruby<E>(
&mut self,
layout_parent_style: &ComputedValues,
flags: CascadeFlags,
) {
use properties::CascadeFlags;
element: Option<E>,
)
where
E: TElement,
{
use properties::computed_value_flags::ComputedValueFlags;
use properties::longhands::unicode_bidi::computed_value::T as UnicodeBidi;
@ -491,7 +511,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
if self.should_suppress_linebreak(layout_parent_style) {
self.style.flags.insert(ComputedValueFlags::SHOULD_SUPPRESS_LINEBREAK);
// Inlinify the display type if allowed.
if !flags.contains(CascadeFlags::SKIP_ROOT_AND_ITEM_BASED_DISPLAY_FIXUP) {
if !self.skip_item_display_fixup(element) {
let inline_display = self_display.inlinify();
if self_display != inline_display {
self.style.mutate_box().set_adjusted_display(inline_display, false);
@ -531,16 +551,22 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
///
/// FIXME(emilio): This isn't technically a style adjustment thingie, could
/// it move somewhere else?
fn adjust_for_visited(&mut self, flags: CascadeFlags) {
use properties::CascadeFlags;
fn adjust_for_visited<E>(&mut self, element: Option<E>)
where
E: TElement,
{
use properties::computed_value_flags::ComputedValueFlags;
if !self.style.has_visited_style() {
return;
}
let relevant_link_visited = if flags.contains(CascadeFlags::IS_LINK) {
flags.contains(CascadeFlags::IS_VISITED_LINK)
let is_link_element =
self.style.pseudo.is_none() &&
element.map_or(false, |e| e.is_link());
let relevant_link_visited = if is_link_element {
element.unwrap().is_visited_link()
} else {
self.style.inherited_flags().contains(ComputedValueFlags::IS_RELEVANT_LINK_VISITED)
};
@ -586,11 +612,35 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
/// When comparing to Gecko, this is similar to the work done by
/// `nsStyleContext::ApplyStyleFixups`, plus some parts of
/// `nsStyleSet::GetContext`.
pub fn adjust(
pub fn adjust<E>(
&mut self,
layout_parent_style: &ComputedValues,
element: Option<E>,
flags: CascadeFlags,
) {
)
where
E: TElement,
{
if cfg!(debug_assertions) {
if element.and_then(|e| e.implemented_pseudo_element()).is_some() {
// It'd be nice to assert `self.style.pseudo == Some(&pseudo)`,
// but we do resolve ::-moz-list pseudos on ::before / ::after
// content, sigh.
debug_assert!(
self.style.pseudo.is_some(),
"Someone really messed up"
);
}
}
// FIXME(emilio): The apply_declarations callsite in Servo's
// animation, and the font stuff for Gecko
// (Stylist::compute_for_declarations) should pass an element to
// cascade(), then we can make this assertion hold everywhere.
// debug_assert!(
// element.is_some() || self.style.pseudo.is_some(),
// "Should always have an element around for non-pseudo styles"
// );
// Don't adjust visited styles, visited-dependent properties aren't
// affected by these adjustments and it'd be just wasted work anyway.
//
@ -600,14 +650,14 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
return;
}
self.adjust_for_visited(flags);
self.adjust_for_visited(element);
#[cfg(feature = "gecko")]
{
self.adjust_for_prohibited_display_contents();
self.adjust_for_fieldset_content(layout_parent_style);
}
self.adjust_for_top_layer();
self.blockify_if_necessary(layout_parent_style, flags);
self.blockify_if_necessary(layout_parent_style, element);
self.adjust_for_position();
self.adjust_for_overflow();
#[cfg(feature = "gecko")]
@ -627,7 +677,7 @@ impl<'a, 'b: 'a> StyleAdjuster<'a, 'b> {
self.adjust_for_text_decoration_lines(layout_parent_style);
#[cfg(feature = "gecko")]
{
self.adjust_for_ruby(layout_parent_style, flags);
self.adjust_for_ruby(layout_parent_style, element);
}
#[cfg(feature = "servo")]
{