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

Support :scope pseudo-class #18790

Merged
merged 5 commits into from Oct 10, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 7 additions & 2 deletions components/script/dom/element.rs
Expand Up @@ -87,6 +87,7 @@ use net_traits::request::CorsSettings;
use ref_filter_map::ref_filter_map;
use script_layout_interface::message::ReflowGoal;
use script_thread::ScriptThread;
use selectors::Element as SelectorsElement;
use selectors::attr::{AttrSelectorOperation, NamespaceConstraint, CaseSensitivity};
use selectors::matching::{ElementSelectorFlags, LocalMatchingContext, MatchingContext, MatchingMode};
use selectors::matching::{HAS_EDGE_CHILD_SELECTOR, HAS_SLOW_SELECTOR, HAS_SLOW_SELECTOR_LATER_SIBLINGS};
Expand Down Expand Up @@ -2188,10 +2189,12 @@ impl ElementMethods for Element {
Err(_) => Err(Error::Syntax),
Ok(selectors) => {
let quirks_mode = document_from_node(self).quirks_mode();
let root = DomRoot::from_ref(self);
// FIXME(bholley): Consider an nth-index cache here.
let mut ctx = MatchingContext::new(MatchingMode::Normal, None, None,
quirks_mode);
Ok(matches_selector_list(&selectors, &DomRoot::from_ref(self), &mut ctx))
ctx.scope_element = Some(root.opaque());
Ok(matches_selector_list(&selectors, &root, &mut ctx))
}
}
}
Expand All @@ -2206,13 +2209,15 @@ impl ElementMethods for Element {
match SelectorParser::parse_author_origin_no_namespace(&selectors) {
Err(_) => Err(Error::Syntax),
Ok(selectors) => {
let self_root = DomRoot::from_ref(self);
let root = self.upcast::<Node>();
for element in root.inclusive_ancestors() {
if let Some(element) = DomRoot::downcast::<Element>(element) {
let quirks_mode = document_from_node(self).quirks_mode();
// FIXME(bholley): Consider an nth-index cache here.
let mut ctx = MatchingContext::new(MatchingMode::Normal, None, None,
quirks_mode);
ctx.scope_element = Some(self_root.opaque());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can hoist the whole definition of ctx up if you want. Anyway, r=me

if matches_selector_list(&selectors, &element, &mut ctx) {
return Ok(Some(element));
}
Expand Down Expand Up @@ -2497,7 +2502,7 @@ impl VirtualMethods for Element {
}
}

impl<'a> ::selectors::Element for DomRoot<Element> {
impl<'a> SelectorsElement for DomRoot<Element> {
type Impl = SelectorImpl;

fn opaque(&self) -> ::selectors::OpaqueElement {
Expand Down
2 changes: 1 addition & 1 deletion components/selectors/builder.rs
Expand Up @@ -284,7 +284,7 @@ fn complex_selector_specificity<Impl>(mut iter: slice::Iter<Component<Impl>>)

Component::FirstChild | Component::LastChild |
Component::OnlyChild | Component::Root |
Component::Empty |
Component::Empty | Component::Scope |
Component::NthChild(..) |
Component::NthLastChild(..) |
Component::NthOfType(..) |
Expand Down
15 changes: 15 additions & 0 deletions components/selectors/context.rs
Expand Up @@ -5,6 +5,7 @@
use attr::CaseSensitivity;
use bloom::BloomFilter;
use nth_index_cache::NthIndexCache;
use tree::OpaqueElement;

/// What kind of selector matching mode we should use.
///
Expand Down Expand Up @@ -88,6 +89,19 @@ pub struct MatchingContext<'a> {
/// only.)
pub relevant_link_found: bool,

/// The element which is going to match :scope pseudo-class. It can be
/// either one :scope element, or the scoping element.
///
/// Note that, although in theory there can be multiple :scope elements,
/// in current specs, at most one is specified, and when there is one,
/// scoping element is not relevant anymore, so we use a single field for
/// them.
///
/// When this is None, :scope will match the root element.
///
/// See https://drafts.csswg.org/selectors-4/#scope-pseudo
pub scope_element: Option<OpaqueElement>,

quirks_mode: QuirksMode,
classes_and_ids_case_sensitivity: CaseSensitivity,
}
Expand Down Expand Up @@ -125,6 +139,7 @@ impl<'a> MatchingContext<'a> {
quirks_mode,
relevant_link_found: false,
classes_and_ids_case_sensitivity: quirks_mode.classes_and_ids_case_sensitivity(),
scope_element: None,
}
}

Expand Down
6 changes: 6 additions & 0 deletions components/selectors/matching.rs
Expand Up @@ -724,6 +724,12 @@ fn matches_simple_selector<E, F>(
flags_setter(element, HAS_EMPTY_SELECTOR);
element.is_empty()
}
Component::Scope => {
match context.shared.scope_element {
Some(ref scope_element) => element.opaque() == *scope_element,
None => element.is_root(),
}
}
Component::NthChild(a, b) => {
matches_generic_nth_child(element, context, a, b, false, false, flags_setter)
}
Expand Down
3 changes: 3 additions & 0 deletions components/selectors/parser.rs
Expand Up @@ -673,6 +673,7 @@ pub enum Component<Impl: SelectorImpl> {
FirstChild, LastChild, OnlyChild,
Root,
Empty,
Scope,
NthChild(i32, i32),
NthLastChild(i32, i32),
NthOfType(i32, i32),
Expand Down Expand Up @@ -969,6 +970,7 @@ impl<Impl: SelectorImpl> ToCss for Component<Impl> {
OnlyChild => dest.write_str(":only-child"),
Root => dest.write_str(":root"),
Empty => dest.write_str(":empty"),
Scope => dest.write_str(":scope"),
FirstOfType => dest.write_str(":first-of-type"),
LastOfType => dest.write_str(":last-of-type"),
OnlyOfType => dest.write_str(":only-of-type"),
Expand Down Expand Up @@ -1699,6 +1701,7 @@ fn parse_simple_pseudo_class<'i, P, E, Impl>(parser: &P, name: CowRcStr<'i>)
"only-child" => Ok(Component::OnlyChild),
"root" => Ok(Component::Root),
"empty" => Ok(Component::Empty),
"scope" => Ok(Component::Scope),
"first-of-type" => Ok(Component::FirstOfType),
"last-of-type" => Ok(Component::LastOfType),
"only-of-type" => Ok(Component::OnlyOfType),
Expand Down
1 change: 1 addition & 0 deletions ports/geckolib/glue.rs
Expand Up @@ -1530,6 +1530,7 @@ pub unsafe extern "C" fn Servo_SelectorList_Matches(
None,
element.owner_document_quirks_mode(),
);
context.scope_element = Some(element.opaque());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fix this too in components/script/dom/element.rs?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I... just tried... but I don't know how to fix it there... dom::script::Element doesn't implement selectors::Element so I cannot just call self.opaque() there, and it's not clear to me how can I get DomRoot<Element> from self there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems that DomRoot::from_ref(self) is what I need here... but you definitely should recheck that I'm doing it the right way.


selectors::matching::matches_selector_list(selectors, &element, &mut context)
}
Expand Down
18 changes: 17 additions & 1 deletion tests/wpt/metadata/MANIFEST.json
Expand Up @@ -152045,6 +152045,18 @@
{}
]
],
"css/selectors4/scope-without-scoping.html": [
[
"/css/selectors4/scope-without-scoping.html",
[
[
"/css/reference/ref-filled-green-100px-square.xht",
"=="
]
],
{}
]
],
"css/selectors4/selector-required.html": [
[
"/css/selectors4/selector-required.html",
Expand Down Expand Up @@ -538672,6 +538684,10 @@
"607553f41a33ce3630752cdf027c9f904833a19d",
"reftest"
],
"css/selectors4/scope-without-scoping.html": [
"f70b8d60543c5a28fcf955b1780f15c03d60991a",
"reftest"
],
"css/selectors4/selector-required-ref.html": [
"815bc765614b4c2e3d8f8f6303e6bb2ee0989c23",
"support"
Expand Down Expand Up @@ -570101,7 +570117,7 @@
"testharness"
],
"dom/nodes/Element-closest.html": [
"4171fb8b70948ba2617e05b118aaf5d9367e916f",
"5abddb81959019267d8b69002ee95b011b2fe34a",
"testharness"
],
"dom/nodes/Element-firstElementChild-entity-xhtml.xhtml": [
Expand Down
3 changes: 3 additions & 0 deletions tests/wpt/metadata/dom/nodes/Element-closest.html.ini
Expand Up @@ -4,3 +4,6 @@
bug: https://github.com/servo/servo/issues/10781
expected: FAIL

[Element.closest with context node 'test4' and selector ':has(> :scope)']
expected: FAIL

@@ -0,0 +1,23 @@
<!DOCTYPE html>
<html lang="en">
<meta charset="UTF-8">
<title>Selectors Level 4: :scope without scoping</title>
<link rel="author" title="Xidorn Quan" href="mailto:me@upsuper.org">
<link rel="author" title="Mozilla" href="https://www.mozilla.org">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this need a <link rel="help"> to pass the CSS lint?

<link rel="help" href="https://drafts.csswg.org/selectors-4/#scope-pseudo">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<style>
div {
width: 100px;
height: 100px;
background: red;
}
:scope > body > div {
background: green;
}
</style>
<body>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div></div>
</body>
</html>
18 changes: 4 additions & 14 deletions tests/wpt/web-platform-tests/dom/nodes/Element-closest.html
Expand Up @@ -56,11 +56,10 @@
do_test(":first-child" , "test12", "test3");
do_test(":invalid" , "test11", "test2");

do_scope_test(":scope" , "test4");
do_scope_test("select > :scope" , "test4");
do_scope_test("div > :scope" , "test4");
do_scope_test(":has(> :scope)" , "test4");

do_test(":scope" , "test4", "test4");
do_test("select > :scope" , "test4", "test4");
do_test("div > :scope" , "test4", "");
do_test(":has(> :scope)" , "test4", "test3");
function do_test(aSelector, aElementId, aTargetId) {
test(function() {
var el = document.getElementById(aElementId).closest(aSelector);
Expand All @@ -71,13 +70,4 @@
}
}, "Element.closest with context node '" + aElementId + "' and selector '" + aSelector + "'");
}

function do_scope_test(aSelector, aElementId) {
test(function() {
var el = document.getElementById(aElementId);
assert_throws("SYNTAX_ERR", function() {
el.closest(aSelector);
});
}, "Element.closest with context node '" + aElementId + "' and selector '" + aSelector + "' should throw");
}
</script>