Auto merge of #22771 - MaT1g3R:refactor_follow_hyperlink, r=jdm

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.

<!-- Please describe your changes on the following line: -->

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `___` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix #22761 (GitHub issue number if applicable)

<!-- Either: -->
- [X] There are tests for these changes

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/22771)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-01-30 08:22:46 -05:00 committed by GitHub
commit 65370f17c9
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 25 additions and 44 deletions

View file

@ -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;
@ -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::<Element>().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 <img ismap/>.
let element = self.upcast::<Element>();
let element = self.as_element();
let mouse_event = event.downcast::<MouseEvent>().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::<Element>() {
if target.is::<HTMLImageElement>() && element.has_attribute(&local_name!("ismap")) {
let target_node = element.upcast::<Node>();
@ -575,16 +571,9 @@ impl Activatable for HTMLAnchorElement {
}
}
// Step 4.
// Step 2.
//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,
};
follow_hyperlink(element, ismap_suffix, referrer_policy);
follow_hyperlink(element, ismap_suffix);
}
//TODO:https://html.spec.whatwg.org/multipage/#the-a-element
@ -599,12 +588,11 @@ impl Activatable for HTMLAnchorElement {
}
/// <https://html.spec.whatwg.org/multipage/#following-hyperlinks-2>
pub fn follow_hyperlink(
subject: &Element,
hyperlink_suffix: Option<String>,
referrer_policy: Option<ReferrerPolicy>,
) {
// Step 1: TODO: If subject cannot navigate, then return.
pub fn follow_hyperlink(subject: &Element, hyperlink_suffix: Option<String>) {
// Step 1.
if subject.cannot_navigate() {
return;
}
// Step 2, done in Step 7.
let document = document_from_node(subject);
@ -631,13 +619,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.
@ -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<Attr>| (determine_policy_for_token(&attribute.Value())));
// Step 13, 14.
debug!("following hyperlink to {}", url);
target_window.load_url(url, replace, false, referrer_policy);

View file

@ -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;
@ -16,12 +15,11 @@ 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;
use html5ever::{LocalName, Prefix};
use net_traits::ReferrerPolicy;
use std::default::Default;
use std::f32;
use std::str;
@ -332,18 +330,6 @@ 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::<Element>(), None, referrer_policy);
follow_hyperlink(self.as_element(), None);
}
}