From e4ffd164495cada2f37741c3ce674d990103c34b Mon Sep 17 00:00:00 2001 From: Peijun Ma Date: Sun, 27 Jan 2019 18:05:37 -0500 Subject: [PATCH] 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); } }