Skip to content

Commit

Permalink
src/node/selector.rs: revise
Browse files Browse the repository at this point in the history
  • Loading branch information
niklak committed Jan 15, 2025
1 parent ee18947 commit 68807b5
Show file tree
Hide file tree
Showing 4 changed files with 19 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/dom_tree/traversal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,8 +103,8 @@ impl Traversal {
let mut candidates = vec![];

while let Some(node_id) = ops.pop() {
// Since these nodes are descendants of the primary node and
// were previously extracted from the `Tree` with only elements remaining,
// Since these nodes are descendants of the primary node and
// were previously extracted from the `Tree` with only elements remaining,
// `else` case should be unreachable.
let Some(node_name) = nodes
.get(node_id.value)
Expand Down
2 changes: 1 addition & 1 deletion src/node/node_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ impl Element {
pub fn has_class(&self, class: &str) -> bool {
self.attrs
.iter()
.find(|attr| &attr.name.local == "class")
.find(|a| a.name.local == local_name!("class"))
.map_or(false, |attr| contains_class(&attr.value, class))
}

Expand Down
15 changes: 11 additions & 4 deletions src/node/selector.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::ops::Deref;

use html5ever::{namespace_url, ns};
use html5ever::{local_name, namespace_url, ns};
use selectors::attr::{AttrSelectorOperation, CaseSensitivity, NamespaceConstraint};
use selectors::context::MatchingContext;
use selectors::matching::ElementSelectorFlags;
Expand Down Expand Up @@ -79,6 +79,7 @@ impl selectors::Element for NodeRef<'_> {

#[inline]
fn has_local_name(&self, local_name: &<Self::Impl as SelectorImpl>::BorrowedLocalName) -> bool {
// This is the most crucial moment for querying.
self.query_or(false, |node| {
if let NodeData::Element(ref e) = node.data {
return &e.name.local == local_name.deref();
Expand Down Expand Up @@ -135,6 +136,7 @@ impl selectors::Element for NodeRef<'_> {
_context: &mut MatchingContext<Self::Impl>,
) -> bool {
use self::NonTSPseudoClass::*;
// TODO: this also can be "optimized", but it's not worth it
match pseudo {
Active | Focus | Hover | Enabled | Disabled | Checked | Indeterminate | Visited => {
false
Expand All @@ -161,10 +163,15 @@ impl selectors::Element for NodeRef<'_> {

/// Whether this element is a `link`.
fn is_link(&self) -> bool {
// TODO: This function introduces significant overhead.
// Its purpose in dom_query is unclear.
// Returning `false` works just fine.
self.query_or(false, |node| {
if let NodeData::Element(ref e) = node.data {
let node_name = e.name.local.as_ref();
return matches!(node_name, "a" | "area" | "link") && e.has_attr("href");
return matches!(
e.name.local,
local_name!("a") | local_name!("area") | local_name!("link")
) && e.attrs.iter().any(|a| a.name.local == local_name!("href"));
}
false
})
Expand All @@ -183,7 +190,7 @@ impl selectors::Element for NodeRef<'_> {
self.query_or(false, |node| {
if let NodeData::Element(ref e) = node.data {
return e.attrs.iter().any(|attr| {
attr.name.local.deref() == "id"
attr.name.local == local_name!("id")
&& case_sensitivity.eq(name.as_bytes(), attr.value.as_bytes())
});
}
Expand Down
12 changes: 5 additions & 7 deletions tests/selection-query.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,13 @@ fn test_is_has() {
assert!(prev_sel.is("*:has( > img:only-child)"));
}


#[cfg_attr(not(target_arch = "wasm32"), test)]
#[cfg_attr(target_arch = "wasm32", wasm_bindgen_test)]
fn test_selection_unique() {
// This document contains many nested div elements and was taken from the dom_smoothie test data.
// I was investigating whether an additional uniqueness check during selection was necessary,
// as the results looked correct and unique without it, while the check added overhead.
// However, after removing the `set.contains` check from `Matches::next`, the dom_smoothie::Readability tests started failing.
// This document contains many nested div elements and was taken from the dom_smoothie test data.
// I was investigating whether an additional uniqueness check during selection was necessary,
// as the results looked correct and unique without it, while the check added overhead.
// However, after removing the `set.contains` check from `Matches::next`, the dom_smoothie::Readability tests started failing.
// Therefore, the current `Matches` implementation requires the uniqueness check despite the overhead.

let contents = include_str!("../test-pages/002.html");
Expand All @@ -177,5 +176,4 @@ fn test_selection_unique() {

let unique_ids = sel_ids.iter().cloned().collect::<HashSet<_>>();
assert_eq!(sel_ids.len(), unique_ids.len());

}
}

0 comments on commit 68807b5

Please sign in to comment.