From 7ed3995725919e02f729a6f5ae83ff7bfba0366b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 15 Oct 2018 00:33:43 +0200 Subject: [PATCH 1/2] style: Stop using PseudoElement::inherits_all. 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. --- components/style/servo/selector_parser.rs | 38 ++--------------------- resources/servo.css | 35 ++++++++++++++++++++- 2 files changed, 37 insertions(+), 36 deletions(-) diff --git a/components/style/servo/selector_parser.rs b/components/style/servo/selector_parser.rs index 7c1e082022d..7b2c06019a4 100644 --- a/components/style/servo/selector_parser.rs +++ b/components/style/servo/selector_parser.rs @@ -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 @@ -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()))) } diff --git a/resources/servo.css b/resources/servo.css index d883e0eccb0..bb6a356e7c5 100644 --- a/resources/servo.css +++ b/resources/servo.css @@ -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; @@ -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; } + +*/ From 6d7d1e56912786a2603613da445ac023c49ddc3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Mon, 15 Oct 2018 00:36:25 +0200 Subject: [PATCH 2/2] style: Remove useless declarations in servo.css. Margin is a reset property, there's no point in setting it to zero since it's zero by default. --- resources/servo.css | 6 ------ 1 file changed, 6 deletions(-) diff --git a/resources/servo.css b/resources/servo.css index bb6a356e7c5..723be706a27 100644 --- a/resources/servo.css +++ b/resources/servo.css @@ -197,16 +197,10 @@ svg > * { /* 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;