From d67d6bfa3a8d6054eed66c575cf97987e8f2ecec Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 18 Apr 2015 16:51:57 +0200 Subject: [PATCH 1/5] Give get_attr_for_layout a more useful signature. --- components/script/dom/element.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 24eda14cab5..9a14157ac60 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -171,14 +171,14 @@ pub trait RawLayoutElementHelpers { #[inline] #[allow(unsafe_code)] -unsafe fn get_attr_for_layout<'a>(elem: &'a Element, namespace: &Namespace, name: &Atom) -> Option<&'a JS> { +unsafe fn get_attr_for_layout(elem: &Element, namespace: &Namespace, name: &Atom) -> Option> { // cast to point to T in RefCell directly let attrs = elem.attrs.borrow_for_layout(); attrs.iter().find(|attr: & &JS| { let attr = attr.to_layout().unsafe_get(); *name == (*attr).local_name_atom_forever() && (*attr).namespace() == namespace - }) + }).map(|attr| attr.to_layout()) } #[allow(unsafe_code)] @@ -187,8 +187,7 @@ impl RawLayoutElementHelpers for Element { unsafe fn get_attr_val_for_layout<'a>(&'a self, namespace: &Namespace, name: &Atom) -> Option<&'a str> { get_attr_for_layout(self, namespace, name).map(|attr| { - let attr = attr.to_layout().unsafe_get(); - (*attr).value_ref_forever() + (*attr.unsafe_get()).value_ref_forever() }) } From 39e3ace817572771d9df38ec125f754f800065b5 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 18 Apr 2015 16:53:05 +0200 Subject: [PATCH 2/5] Use get_attr_for_layout in get_attr_atom_for_layout. The code is equivalent. --- components/script/dom/element.rs | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 9a14157ac60..09c5232a264 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -207,14 +207,8 @@ impl RawLayoutElementHelpers for Element { #[inline] unsafe fn get_attr_atom_for_layout(&self, namespace: &Namespace, name: &Atom) -> Option { - let attrs = self.attrs.borrow_for_layout(); - (*attrs).iter().find(|attr: & &JS| { - let attr = attr.to_layout().unsafe_get(); - *name == (*attr).local_name_atom_forever() && - (*attr).namespace() == namespace - }).and_then(|attr| { - let attr = attr.to_layout().unsafe_get(); - (*attr).value_atom_forever() + get_attr_for_layout(self, namespace, name).and_then(|attr| { + (*attr.unsafe_get()).value_atom_forever() }) } From bdb8657cf68307249607b6abe9723c8577ea7621 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 18 Apr 2015 16:56:54 +0200 Subject: [PATCH 3/5] Use get_attr_for_layout in ghas_class_for_layout. This fixes a panic if this code was ever called on an element with a class attribute in a non-null namespace. In this case, the attribute would not have been parsed into a list of tokens, so value_tokens_forever() would have returned None. However, this function is, as far as I can tell, never called, because of the way selectors are evaluated in layout. ('Return the selectors that match this node' rather than 'return the nodes that match this selector'; the latter uses only each_class.) --- components/script/dom/element.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 09c5232a264..06eae2592c7 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -214,16 +214,9 @@ impl RawLayoutElementHelpers for Element { #[inline] unsafe fn has_class_for_layout(&self, name: &Atom) -> bool { - let attrs = self.attrs.borrow_for_layout(); - (*attrs).iter().find(|attr: & &JS| { - let attr = attr.to_layout().unsafe_get(); - (*attr).local_name_atom_forever() == atom!("class") - }).map_or(false, |attr| { - let attr = attr.to_layout().unsafe_get(); - (*attr).value_tokens_forever().map(|tokens| { - tokens.iter().any(|atom| atom == name) - }) - }.take().unwrap()) + get_attr_for_layout(self, &ns!(""), &atom!("class")).map_or(false, |attr| { + (*attr.unsafe_get()).value_tokens_forever().unwrap().iter().any(|atom| atom == name) + }) } #[inline] From 21d3ddabbf1ad51492d730bac3a838b78c1f6f17 Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 18 Apr 2015 17:34:19 +0200 Subject: [PATCH 4/5] Add a test for the class selector with multiple class attributes. --- tests/wpt/mozilla/meta/MANIFEST.json | 29 ++++++++++++++++++- .../meta/css/class-namespaces.html.ini | 3 ++ .../tests/css/class-namespaces-ref.html | 5 ++++ .../mozilla/tests/css/class-namespaces.html | 16 ++++++++++ 4 files changed, 52 insertions(+), 1 deletion(-) create mode 100644 tests/wpt/mozilla/meta/css/class-namespaces.html.ini create mode 100644 tests/wpt/mozilla/tests/css/class-namespaces-ref.html create mode 100644 tests/wpt/mozilla/tests/css/class-namespaces.html diff --git a/tests/wpt/mozilla/meta/MANIFEST.json b/tests/wpt/mozilla/meta/MANIFEST.json index de093bd00a2..efd18936509 100644 --- a/tests/wpt/mozilla/meta/MANIFEST.json +++ b/tests/wpt/mozilla/meta/MANIFEST.json @@ -14,6 +14,20 @@ "local_changes": { "deleted": [], "items": { + "reftest": { + "css/class-namespaces.html": [ + { + "path": "css/class-namespaces.html", + "references": [ + [ + "/_mozilla/css/class-namespaces-ref.html", + "==" + ] + ], + "url": "/_mozilla/css/class-namespaces.html" + } + ] + }, "testharness": { "mozilla/DOMParser.html": [ { @@ -515,7 +529,20 @@ ] } }, - "reftest_nodes": {} + "reftest_nodes": { + "css/class-namespaces.html": [ + { + "path": "css/class-namespaces.html", + "references": [ + [ + "/_mozilla/css/class-namespaces-ref.html", + "==" + ] + ], + "url": "/_mozilla/css/class-namespaces.html" + } + ] + } }, "reftest_nodes": {}, "rev": null, diff --git a/tests/wpt/mozilla/meta/css/class-namespaces.html.ini b/tests/wpt/mozilla/meta/css/class-namespaces.html.ini new file mode 100644 index 00000000000..abf49ed322c --- /dev/null +++ b/tests/wpt/mozilla/meta/css/class-namespaces.html.ini @@ -0,0 +1,3 @@ +[class-namespaces.html] + type: reftest + expected: FAIL diff --git a/tests/wpt/mozilla/tests/css/class-namespaces-ref.html b/tests/wpt/mozilla/tests/css/class-namespaces-ref.html new file mode 100644 index 00000000000..96c68eff366 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/class-namespaces-ref.html @@ -0,0 +1,5 @@ + + +

AAA

BBB

diff --git a/tests/wpt/mozilla/tests/css/class-namespaces.html b/tests/wpt/mozilla/tests/css/class-namespaces.html new file mode 100644 index 00000000000..239d1629fc3 --- /dev/null +++ b/tests/wpt/mozilla/tests/css/class-namespaces.html @@ -0,0 +1,16 @@ + + + + From 0042f78aa71d20914ee29b077510b9944401aaef Mon Sep 17 00:00:00 2001 From: Ms2ger Date: Sat, 18 Apr 2015 16:58:17 +0200 Subject: [PATCH 5/5] Use get_attr_for_layout in get_classes_for_layout. This fixes a bug with elements with multiple class attributes. In this case, the class attribute in the null namespace would only be considered if it was the first class attribute in the list. --- components/script/dom/element.rs | 9 ++------- tests/wpt/mozilla/meta/css/class-namespaces.html.ini | 3 --- 2 files changed, 2 insertions(+), 10 deletions(-) delete mode 100644 tests/wpt/mozilla/meta/css/class-namespaces.html.ini diff --git a/components/script/dom/element.rs b/components/script/dom/element.rs index 06eae2592c7..88366f591d2 100644 --- a/components/script/dom/element.rs +++ b/components/script/dom/element.rs @@ -221,13 +221,8 @@ impl RawLayoutElementHelpers for Element { #[inline] unsafe fn get_classes_for_layout(&self) -> Option<&'static [Atom]> { - let attrs = self.attrs.borrow_for_layout(); - (*attrs).iter().find(|attr: & &JS| { - let attr = attr.to_layout().unsafe_get(); - (*attr).local_name_atom_forever() == atom!("class") - }).and_then(|attr| { - let attr = attr.to_layout().unsafe_get(); - (*attr).value_tokens_forever() + get_attr_for_layout(self, &ns!(""), &atom!("class")).map(|attr| { + (*attr.unsafe_get()).value_tokens_forever().unwrap() }) } diff --git a/tests/wpt/mozilla/meta/css/class-namespaces.html.ini b/tests/wpt/mozilla/meta/css/class-namespaces.html.ini deleted file mode 100644 index abf49ed322c..00000000000 --- a/tests/wpt/mozilla/meta/css/class-namespaces.html.ini +++ /dev/null @@ -1,3 +0,0 @@ -[class-namespaces.html] - type: reftest - expected: FAIL