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

Generate ::before and ::after content for layout_2020 #25817

Merged
merged 5 commits into from Feb 25, 2020

Generate ::before and ::after string content for layout 2020

  • Loading branch information
ferjm committed Feb 25, 2020
commit bc66948f7cf7b1dcce3cca7af7b4a657ccd5806d
@@ -17,8 +17,10 @@ use std::sync::Arc;
use style::dom::{OpaqueNode, TNode};
use style::properties::ComputedValues;
use style::selector_parser::PseudoElement;
use style::values::generics::counters::Content;
use style::values::generics::counters::ContentItem;

#[derive(Clone, Copy)]
#[derive(Clone, Copy, Debug)]
pub enum WhichPseudoElement {
Before,
After,
@@ -244,29 +246,51 @@ impl NonReplacedContents {
}

fn pseudo_element_style<'dom, Node>(
_which: WhichPseudoElement,
_element: Node,
_context: &LayoutContext,
which: WhichPseudoElement,
element: Node,
context: &LayoutContext,
) -> Option<ServoArc<ComputedValues>>
where
Node: NodeExt<'dom>,
{
// FIXME: run the cascade, then return None for `content: normal` or `content: none`
// https://drafts.csswg.org/css2/generate.html#content
None
if let Some(pseudo_element) = match which {
WhichPseudoElement::Before => element.to_threadsafe().get_before_pseudo(),
WhichPseudoElement::After => element.to_threadsafe().get_after_pseudo(),
} {
let style = pseudo_element.style(context.shared_context());
if style.ineffective_content_property() {
None
} else {
Some(style)
}
} else {
None
}
This conversation was marked as resolved by SimonSapin
Comment on lines +266 to +268

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Feb 20, 2020

Member

Nit: does this if let…else look better as .and_then(|| {…})?

}

fn generate_pseudo_element_content<'dom, Node>(
_pseudo_element_style: &ComputedValues,
pseudo_element_style: &ComputedValues,
_element: Node,
_context: &LayoutContext,
) -> Vec<PseudoElementContentItem>
where
Node: NodeExt<'dom>,
{
let _ = PseudoElementContentItem::Text;
let _ = PseudoElementContentItem::Replaced;
unimplemented!()
match &pseudo_element_style.get_counters().content {
Content::Items(ref items) => {
let mut vec = vec![];
for item in items.iter() {
match item {
ContentItem::String(s) => {
vec.push(PseudoElementContentItem::Text(s.to_string()))
},
_ => unimplemented!(),
This conversation was marked as resolved by SimonSapin

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Feb 20, 2020

Member

Maybe silently ignoring unsupported values is better than panicking. I wonder if panics cause reftests to timeout. (This would be another bug that we should fix, but oh well.)

For full spec compliance and for making @supports meaningful we should not parse values that are not supported yet, but no need to bother if we’re adding support soon.

}
}
vec
},
_ => vec![],
This conversation was marked as resolved by SimonSapin

This comment has been minimized.

Copy link
@SimonSapin

SimonSapin Feb 20, 2020

Member
Suggested change
_ => vec![],
Content::Normal | Content::None => unreachable!(),

Here however this function should not be called with these values (because pseudo_element_style returned Option::None), so panicking is appropriate since it indicates a bug.

}
}

pub struct BoxSlot<'dom> {
@@ -11,7 +11,6 @@ ${helpers.predefined_type(
"Content",
"computed::Content::normal()",
engines="gecko servo-2013 servo-2020",
servo_2020_pref="layout.2020.unimplemented",
initial_specified_value="specified::Content::normal()",
animation_value_type="discrete",
spec="https://drafts.csswg.org/css-content/#propdef-content",
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.