Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

style: Stop using PseudoElement::inherits_all. #21946

Merged
merged 2 commits into from Oct 15, 2018
Merged
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -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.

This comment has been minimized.

@nox

nox Oct 15, 2018

Member

Why do we keep it then?

This comment has been minimized.

@emilio

emilio Oct 15, 2018

Author Member

Because I'm removing it in that Gecko bug I point above, which I'll sink in a few..

This comment has been minimized.

@emilio

emilio Oct 15, 2018

Author Member

sync*

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
@@ -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())))
}
@@ -168,18 +168,39 @@ 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;
text-overflow: inherit;
overflow: inherit;
}

/* style for text in input elements. */
*|*::-servo-input-text {
margin: 0;
}

*|*::-servo-table-wrapper {
display: table;
border: none;
@@ -239,10 +260,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;
}
*/
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.