From e4ffd164495cada2f37741c3ce674d990103c34b Mon Sep 17 00:00:00 2001 From: Peijun Ma Date: Sun, 27 Jan 2019 18:05:37 -0500 Subject: [PATCH 1/3] Refactor follow_hyperlink and its callers Now they follow the new spec stated at: https://html.spec.whatwg.org/multipage/links.html#following-hyperlinks-2 It seems like choosing a browsing context is already done in the follow_hyperlink method, so I have removed the TODO in activation_behavior for HTMLAreaElement. The tests in tests/wpt/web-platform-tests/html/semantics/links/following-hyperlinks/ pass in release builds, but still don't pass in dev build, since the timeout in tests/wpt/web-platform-tests/html/semantics/links/following-hyperlinks/activation-behavior.window.js seems to be too short for dev builds. Navigating to error page on failed URL parsing is still not implemented. There seem to be potential code duplication in activation_behavior methods for both htmlanchorelement.rs and htmlareaelement.rs, in: let referrer_policy = match self.RelList().Contains("noreferrer".into()) { true => Some(ReferrerPolicy::NoReferrer), false => None, }; I didn't pull them out to a separate function since I don't know where I would put that new function. --- components/script/dom/htmlanchorelement.rs | 28 +++++++++++----------- components/script/dom/htmlareaelement.rs | 12 ++-------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index e024fb6a00a..748ed682f2c 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/components/script/dom/htmlanchorelement.rs @@ -541,7 +541,7 @@ impl Activatable for HTMLAnchorElement { // hyperlink" // https://html.spec.whatwg.org/multipage/#the-a-element // "The activation behaviour of a elements *that create hyperlinks*" - self.upcast::().has_attribute(&local_name!("href")) + self.as_element().has_attribute(&local_name!("href")) } //TODO:https://html.spec.whatwg.org/multipage/#the-a-element @@ -553,16 +553,12 @@ impl Activatable for HTMLAnchorElement { //https://html.spec.whatwg.org/multipage/#the-a-element:activation-behaviour fn activation_behavior(&self, event: &Event, target: &EventTarget) { - //Step 1. If the node document is not fully active, abort. - let doc = document_from_node(self); - if !doc.is_fully_active() { - return; - } - //TODO: Step 2. Check if browsing context is specified and act accordingly. - //Step 3. Handle . - let element = self.upcast::(); + let element = self.as_element(); let mouse_event = event.downcast::().unwrap(); let mut ismap_suffix = None; + + // Step 1: If the target of the click event is an img element with an ismap attribute + // specified, then server-side image map processing must be performed. if let Some(element) = target.downcast::() { if target.is::() && element.has_attribute(&local_name!("ismap")) { let target_node = element.upcast::(); @@ -575,15 +571,14 @@ impl Activatable for HTMLAnchorElement { } } - // Step 4. - //TODO: Download the link is `download` attribute is set. - // https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery let referrer_policy = match self.RelList().Contains("noreferrer".into()) { true => Some(ReferrerPolicy::NoReferrer), false => None, }; + // Step 2. + //TODO: Download the link is `download` attribute is set. follow_hyperlink(element, ismap_suffix, referrer_policy); } @@ -604,7 +599,10 @@ pub fn follow_hyperlink( hyperlink_suffix: Option, referrer_policy: Option, ) { - // Step 1: TODO: If subject cannot navigate, then return. + // Step 1: If subject cannot navigate, then return. + if subject.cannot_navigate() { + return; + } // Step 2, done in Step 7. let document = document_from_node(subject); @@ -631,13 +629,15 @@ pub fn follow_hyperlink( Some(name) => source.choose_browsing_context(name.Value(), noopener), None => (Some(window.window_proxy()), false), }; + + // Step 8. let chosen = match maybe_chosen { Some(proxy) => proxy, None => return, }; + if let Some(target_document) = chosen.document() { let target_window = target_document.window(); - // Step 9, dis-owning target's opener, if necessary // will have been done as part of Step 7 above // in choose_browsing_context/create_auxiliary_browsing_context. diff --git a/components/script/dom/htmlareaelement.rs b/components/script/dom/htmlareaelement.rs index 32a5e8ef89a..a1cf35a7eeb 100644 --- a/components/script/dom/htmlareaelement.rs +++ b/components/script/dom/htmlareaelement.rs @@ -16,7 +16,7 @@ use crate::dom::event::Event; use crate::dom::eventtarget::EventTarget; use crate::dom::htmlanchorelement::follow_hyperlink; use crate::dom::htmlelement::HTMLElement; -use crate::dom::node::{document_from_node, Node}; +use crate::dom::node::Node; use crate::dom::virtualmethods::VirtualMethods; use dom_struct::dom_struct; use euclid::Point2D; @@ -332,18 +332,10 @@ impl Activatable for HTMLAreaElement { } fn activation_behavior(&self, _event: &Event, _target: &EventTarget) { - // Step 1 - let doc = document_from_node(self); - if !doc.is_fully_active() { - return; - } - // Step 2 - // TODO : We should be choosing a browsing context and navigating to it. - // Step 3 let referrer_policy = match self.RelList().Contains("noreferrer".into()) { true => Some(ReferrerPolicy::NoReferrer), false => None, }; - follow_hyperlink(self.upcast::(), None, referrer_policy); + follow_hyperlink(self.as_element(), None, referrer_policy); } } From fe7f3ab2622424b80b939e758b12fb895e787789 Mon Sep 17 00:00:00 2001 From: Peijun Ma Date: Mon, 28 Jan 2019 01:43:29 -0500 Subject: [PATCH 2/3] Remove description for step in follow_hyperlink --- components/script/dom/htmlanchorelement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index 748ed682f2c..0dedcdf7553 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/components/script/dom/htmlanchorelement.rs @@ -599,7 +599,7 @@ pub fn follow_hyperlink( hyperlink_suffix: Option, referrer_policy: Option, ) { - // Step 1: If subject cannot navigate, then return. + // Step 1. if subject.cannot_navigate() { return; } From 0bafa61f7cfc4a5783828b8f64e819fd715fea63 Mon Sep 17 00:00:00 2001 From: Peijun Ma Date: Wed, 30 Jan 2019 00:23:01 -0500 Subject: [PATCH 3/3] Implement referrer policy for follow_hyperlink --- components/script/dom/htmlanchorelement.rs | 25 +++++++++------------- components/script/dom/htmlareaelement.rs | 8 +------ 2 files changed, 11 insertions(+), 22 deletions(-) diff --git a/components/script/dom/htmlanchorelement.rs b/components/script/dom/htmlanchorelement.rs index 0dedcdf7553..3f078c51071 100644 --- a/components/script/dom/htmlanchorelement.rs +++ b/components/script/dom/htmlanchorelement.rs @@ -3,9 +3,9 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::dom::activation::Activatable; +use crate::dom::attr::Attr; use crate::dom::bindings::cell::DomRefCell; use crate::dom::bindings::codegen::Bindings::AttrBinding::AttrMethods; -use crate::dom::bindings::codegen::Bindings::DOMTokenListBinding::DOMTokenListMethods; use crate::dom::bindings::codegen::Bindings::HTMLAnchorElementBinding; use crate::dom::bindings::codegen::Bindings::HTMLAnchorElementBinding::HTMLAnchorElementMethods; use crate::dom::bindings::codegen::Bindings::MouseEventBinding::MouseEventMethods; @@ -13,6 +13,7 @@ use crate::dom::bindings::codegen::Bindings::NodeBinding::NodeMethods; use crate::dom::bindings::inheritance::Castable; use crate::dom::bindings::root::{DomRoot, MutNullableDom}; use crate::dom::bindings::str::{DOMString, USVString}; +use crate::dom::document::determine_policy_for_token; use crate::dom::document::Document; use crate::dom::domtokenlist::DOMTokenList; use crate::dom::element::Element; @@ -26,7 +27,6 @@ use crate::dom::urlhelper::UrlHelper; use crate::dom::virtualmethods::VirtualMethods; use dom_struct::dom_struct; use html5ever::{LocalName, Prefix}; -use net_traits::ReferrerPolicy; use num_traits::ToPrimitive; use servo_url::ServoUrl; use std::default::Default; @@ -571,15 +571,9 @@ impl Activatable for HTMLAnchorElement { } } - // https://w3c.github.io/webappsec-referrer-policy/#referrer-policy-delivery - let referrer_policy = match self.RelList().Contains("noreferrer".into()) { - true => Some(ReferrerPolicy::NoReferrer), - false => None, - }; - // Step 2. //TODO: Download the link is `download` attribute is set. - follow_hyperlink(element, ismap_suffix, referrer_policy); + follow_hyperlink(element, ismap_suffix); } //TODO:https://html.spec.whatwg.org/multipage/#the-a-element @@ -594,11 +588,7 @@ impl Activatable for HTMLAnchorElement { } /// -pub fn follow_hyperlink( - subject: &Element, - hyperlink_suffix: Option, - referrer_policy: Option, -) { +pub fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option) { // Step 1. if subject.cannot_navigate() { return; @@ -645,7 +635,7 @@ pub fn follow_hyperlink( // Step 10, 11, 12, 13. TODO: if parsing the URL failed, navigate to error page. let attribute = subject.get_attribute(&ns!(), &local_name!("href")).unwrap(); let mut href = attribute.Value(); - // Step 12: append a hyperlink suffix. + // Step 11: append a hyperlink suffix. // https://www.w3.org/Bugs/Public/show_bug.cgi?id=28925 if let Some(suffix) = hyperlink_suffix { href.push_str(&suffix); @@ -655,6 +645,11 @@ pub fn follow_hyperlink( Err(_) => return, }; + // Step 12. + let referrer_policy = subject + .get_attribute_by_name(DOMString::from_string(String::from("referrerpolicy"))) + .and_then(|attribute: DomRoot| (determine_policy_for_token(&attribute.Value()))); + // Step 13, 14. debug!("following hyperlink to {}", url); target_window.load_url(url, replace, false, referrer_policy); diff --git a/components/script/dom/htmlareaelement.rs b/components/script/dom/htmlareaelement.rs index a1cf35a7eeb..37d020e684f 100644 --- a/components/script/dom/htmlareaelement.rs +++ b/components/script/dom/htmlareaelement.rs @@ -3,7 +3,6 @@ * file, You can obtain one at https://mozilla.org/MPL/2.0/. */ use crate::dom::activation::Activatable; -use crate::dom::bindings::codegen::Bindings::DOMTokenListBinding::DOMTokenListMethods; use crate::dom::bindings::codegen::Bindings::HTMLAreaElementBinding; use crate::dom::bindings::codegen::Bindings::HTMLAreaElementBinding::HTMLAreaElementMethods; use crate::dom::bindings::inheritance::Castable; @@ -21,7 +20,6 @@ use crate::dom::virtualmethods::VirtualMethods; use dom_struct::dom_struct; use euclid::Point2D; use html5ever::{LocalName, Prefix}; -use net_traits::ReferrerPolicy; use std::default::Default; use std::f32; use std::str; @@ -332,10 +330,6 @@ impl Activatable for HTMLAreaElement { } fn activation_behavior(&self, _event: &Event, _target: &EventTarget) { - let referrer_policy = match self.RelList().Contains("noreferrer".into()) { - true => Some(ReferrerPolicy::NoReferrer), - false => None, - }; - follow_hyperlink(self.as_element(), None, referrer_policy); + follow_hyperlink(self.as_element(), None); } }