Skip to content

Commit

Permalink
style: Stop using PseudoElement::inherits_all.
Browse files Browse the repository at this point in the history
This was done that way just because Servo didn't support the `all` property at
the time.

We should do it this way and optimize it if it's slow. Though I suspect that
most of stuff doesn't actually need to be inherited, my patch at bug 1498943
should make it much faster than what it would otherwise be.
  • Loading branch information
emilio committed Oct 15, 2018
1 parent 5327758 commit 7ed3995
Show file tree
Hide file tree
Showing 2 changed files with 37 additions and 36 deletions.
38 changes: 3 additions & 35 deletions components/style/servo/selector_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -221,41 +221,9 @@ impl PseudoElement {
}
}

/// For most (but not all) anon-boxes, we inherit all values from the
/// parent, this is the hook in the style system to allow this.
///
/// FIXME(emilio): It's likely that this is broken in a variety of
/// situations, and what it really wants is just inherit some reset
/// properties... Also, I guess it just could do all: inherit on the
/// stylesheet, though chances are that'd be kinda slow if we don't cache
/// them...
/// To be removed.
pub fn inherits_all(&self) -> bool {
match *self {
PseudoElement::After |
PseudoElement::Before |
PseudoElement::Selection |
PseudoElement::DetailsContent |
PseudoElement::DetailsSummary |
// Anonymous table flows shouldn't inherit their parents properties in order
// to avoid doubling up styles such as transformations.
PseudoElement::ServoAnonymousTableCell |
PseudoElement::ServoAnonymousTableRow |
PseudoElement::ServoText |
PseudoElement::ServoInputText => false,

// For tables, we do want style to inherit, because TableWrapper is
// responsible for handling clipping and scrolling, while Table is
// responsible for creating stacking contexts.
//
// StackingContextCollectionFlags makes sure this is processed
// properly.
PseudoElement::ServoAnonymousTable |
PseudoElement::ServoAnonymousTableWrapper |
PseudoElement::ServoTableWrapper |
PseudoElement::ServoAnonymousBlock |
PseudoElement::ServoInlineBlockWrapper |
PseudoElement::ServoInlineAbsolute => true,
}
false
}

/// Covert non-canonical pseudo-element to canonical one, and keep a
Expand Down Expand Up @@ -585,7 +553,7 @@ impl<'a, 'i> ::selectors::Parser<'i> for SelectorParser<'a> {
}
ServoInlineBlockWrapper
},
"-servo-input-absolute" => {
"-servo-inline-absolute" => {
if !self.in_user_agent_stylesheet() {
return Err(location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(name.clone())))
}
Expand Down
35 changes: 34 additions & 1 deletion resources/servo.css
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,33 @@ svg > * {
display: none;
}

/*
* For most (but not all) anon-boxes, we inherit all values from the
* parent.
*
* Anonymous table flows shouldn't inherit their parents properties in order
* to avoid doubling up styles such as transformations. Same for text and such.
*
* For tables, we do want style to inherit, because TableWrapper is
* responsible for handling clipping and scrolling, while Table is
* responsible for creating stacking contexts.
*
* StackingContextCollectionFlags makes sure this is processed
* properly.
*
* FIXME(emilio): inheriting all is a very strong hammer, and it's likely
* broken for stuff like table backgrounds and such. Gecko explicitly inherits
* what it wants, which seems a bit better off-hand.
*/
*|*::-servo-anonymous-table,
*|*::-servo-anonymous-table-wrapper,
*|*::-servo-table-wrapper,
*|*::-servo-anonymous-block,
*|*::-servo-inline-block-wrapper,
*|*::-servo-inline-absolute {
all: inherit;
}

/* style for text node. */
*|*::-servo-text {
margin: 0;
Expand Down Expand Up @@ -239,10 +266,16 @@ svg > * {
margin: 0;
}

/* The outer fragment wrapper of an inline absolute hypothetical fragment. */
/* The outer fragment wrapper of an inline absolute hypothetical fragment.
*
* FIXME(emilio): This was disabled because of a typo in the CSS parser,
* re-enable or figure out why it's not needed.
*|*::-servo-inline-absolute {
clip: auto;
border: none;
padding: 0;
margin: 0;
}
*/

0 comments on commit 7ed3995

Please sign in to comment.