Auto merge of #24142 - CYBAI:warn-module-script, r=jdm

Show warning when module scripts are ignored

The first five commits are `cherry-pick`-ed from the module script PR.
I think it might be also good to have this PR first so that reviewers can focus on module script things on that PR!

---
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors
- [x] These changes fix #24089
- [x] These changes do not require tests because it just ignored module scripts (or should we turn on module script tests in this PR and update those test expectation to TIMEOUT?)

<!-- 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/24142)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2019-09-06 07:36:26 -04:00 committed by GitHub
commit 7a67261dce
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23

View file

@ -125,35 +125,44 @@ static SCRIPT_JS_MIMES: StaticStringVec = &[
"text/x-javascript", "text/x-javascript",
]; ];
#[derive(Clone, Copy, JSTraceable, MallocSizeOf, PartialEq)]
pub enum ScriptType {
Classic,
Module,
}
#[derive(JSTraceable, MallocSizeOf)] #[derive(JSTraceable, MallocSizeOf)]
pub struct ClassicScript { pub struct ScriptOrigin {
text: DOMString, text: DOMString,
url: ServoUrl, url: ServoUrl,
external: bool, external: bool,
type_: ScriptType,
} }
impl ClassicScript { impl ScriptOrigin {
fn internal(text: DOMString, url: ServoUrl) -> ClassicScript { fn internal(text: DOMString, url: ServoUrl, type_: ScriptType) -> ScriptOrigin {
ClassicScript { ScriptOrigin {
text: text, text: text,
url: url, url: url,
external: false, external: false,
type_,
} }
} }
fn external(text: DOMString, url: ServoUrl) -> ClassicScript { fn external(text: DOMString, url: ServoUrl, type_: ScriptType) -> ScriptOrigin {
ClassicScript { ScriptOrigin {
text: text, text: text,
url: url, url: url,
external: true, external: true,
type_,
} }
} }
} }
pub type ScriptResult = Result<ClassicScript, NetworkError>; pub type ScriptResult = Result<ScriptOrigin, NetworkError>;
/// The context required for asynchronously loading an external script source. /// The context required for asynchronously loading an external script source.
struct ScriptContext { struct ClassicContext {
/// The element that initiated the request. /// The element that initiated the request.
elem: Trusted<HTMLScriptElement>, elem: Trusted<HTMLScriptElement>,
/// The kind of external script. /// The kind of external script.
@ -173,7 +182,7 @@ struct ScriptContext {
resource_timing: ResourceFetchTiming, resource_timing: ResourceFetchTiming,
} }
impl FetchResponseListener for ScriptContext { impl FetchResponseListener for ClassicContext {
fn process_request_body(&mut self) {} // TODO(KiChjang): Perhaps add custom steps to perform fetch here? fn process_request_body(&mut self) {} // TODO(KiChjang): Perhaps add custom steps to perform fetch here?
fn process_request_eof(&mut self) {} // TODO(KiChjang): Perhaps add custom steps to perform fetch here? fn process_request_eof(&mut self) {} // TODO(KiChjang): Perhaps add custom steps to perform fetch here?
@ -226,7 +235,11 @@ impl FetchResponseListener for ScriptContext {
// Step 7. // Step 7.
let (source_text, _, _) = encoding.decode(&self.data); let (source_text, _, _) = encoding.decode(&self.data);
ClassicScript::external(DOMString::from(source_text), metadata.final_url) ScriptOrigin::external(
DOMString::from(source_text),
metadata.final_url,
ScriptType::Classic,
)
}); });
// Step 9. // Step 9.
@ -260,7 +273,7 @@ impl FetchResponseListener for ScriptContext {
} }
} }
impl ResourceTimingListener for ScriptContext { impl ResourceTimingListener for ClassicContext {
fn resource_timing_information(&self) -> (InitiatorType, ServoUrl) { fn resource_timing_information(&self) -> (InitiatorType, ServoUrl) {
let initiator_type = InitiatorType::LocalName( let initiator_type = InitiatorType::LocalName(
self.elem self.elem
@ -277,7 +290,7 @@ impl ResourceTimingListener for ScriptContext {
} }
} }
impl PreInvoke for ScriptContext {} impl PreInvoke for ClassicContext {}
/// <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script> /// <https://html.spec.whatwg.org/multipage/#fetch-a-classic-script>
fn fetch_a_classic_script( fn fetch_a_classic_script(
@ -313,7 +326,7 @@ fn fetch_a_classic_script(
// TODO: Step 3, Add custom steps to perform fetch // TODO: Step 3, Add custom steps to perform fetch
let context = Arc::new(Mutex::new(ScriptContext { let context = Arc::new(Mutex::new(ClassicContext {
elem: Trusted::new(script), elem: Trusted::new(script),
kind: kind, kind: kind,
character_encoding: character_encoding, character_encoding: character_encoding,
@ -364,47 +377,49 @@ impl HTMLScriptElement {
self.non_blocking.set(true); self.non_blocking.set(true);
} }
// Step 4. // Step 4-5.
let text = self.Text(); let text = self.Text();
if text.is_empty() && !element.has_attribute(&local_name!("src")) { if text.is_empty() && !element.has_attribute(&local_name!("src")) {
return; return;
} }
// Step 5. // Step 6.
if !self.upcast::<Node>().is_connected() { if !self.upcast::<Node>().is_connected() {
return; return;
} }
// Step 6. let script_type = if let Some(ty) = self.get_script_type() {
if !self.is_javascript() { ty
} else {
// Step 7.
return; return;
} };
// Step 7. // Step 8.
if was_parser_inserted { if was_parser_inserted {
self.parser_inserted.set(true); self.parser_inserted.set(true);
self.non_blocking.set(false); self.non_blocking.set(false);
} }
// Step 8. // Step 9.
self.already_started.set(true); self.already_started.set(true);
// Step 9. // Step 10.
let doc = document_from_node(self); let doc = document_from_node(self);
if self.parser_inserted.get() && &*self.parser_document != &*doc { if self.parser_inserted.get() && &*self.parser_document != &*doc {
return; return;
} }
// Step 10. // Step 11.
if !doc.is_scripting_enabled() { if !doc.is_scripting_enabled() {
return; return;
} }
// TODO: Step 11: nomodule content attribute // TODO: Step 12: nomodule content attribute
// TODO(#4577): Step 12: CSP. // TODO(#4577): Step 13: CSP.
// Step 13. // Step 14.
let for_attribute = element.get_attribute(&ns!(), &local_name!("for")); let for_attribute = element.get_attribute(&ns!(), &local_name!("for"));
let event_attribute = element.get_attribute(&ns!(), &local_name!("event")); let event_attribute = element.get_attribute(&ns!(), &local_name!("event"));
match (for_attribute, event_attribute) { match (for_attribute, event_attribute) {
@ -424,20 +439,20 @@ impl HTMLScriptElement {
(_, _) => (), (_, _) => (),
} }
// Step 14. // Step 15.
let encoding = element let encoding = element
.get_attribute(&ns!(), &local_name!("charset")) .get_attribute(&ns!(), &local_name!("charset"))
.and_then(|charset| Encoding::for_label(charset.value().as_bytes())) .and_then(|charset| Encoding::for_label(charset.value().as_bytes()))
.unwrap_or_else(|| doc.encoding()); .unwrap_or_else(|| doc.encoding());
// Step 15. // Step 16.
let cors_setting = cors_setting_for_element(element); let cors_setting = cors_setting_for_element(element);
// TODO: Step 16: Module script credentials mode. // TODO: Step 17: Module script credentials mode.
// TODO: Step 17: Nonce. // TODO: Step 18: Nonce.
// Step 18: Integrity metadata. // Step 19: Integrity metadata.
let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity")); let im_attribute = element.get_attribute(&ns!(), &local_name!("integrity"));
let integrity_val = im_attribute.as_ref().map(|a| a.value()); let integrity_val = im_attribute.as_ref().map(|a| a.value());
let integrity_metadata = match integrity_val { let integrity_metadata = match integrity_val {
@ -445,26 +460,30 @@ impl HTMLScriptElement {
None => "", None => "",
}; };
// TODO: Step 19: parser state. // TODO: Step 20: referrer policy
// TODO: Step 20: environment settings object. // TODO: Step 21: parser state.
// TODO: Step 22: Fetch options
// TODO: Step 23: environment settings object.
let base_url = doc.base_url(); let base_url = doc.base_url();
if let Some(src) = element.get_attribute(&ns!(), &local_name!("src")) { if let Some(src) = element.get_attribute(&ns!(), &local_name!("src")) {
// Step 21. // Step 24.
// Step 21.1. // Step 24.1.
let src = src.value(); let src = src.value();
// Step 21.2. // Step 24.2.
if src.is_empty() { if src.is_empty() {
self.queue_error_event(); self.queue_error_event();
return; return;
} }
// Step 21.3: The "from an external file"" flag is stored in ClassicScript. // Step 24.3: The "from an external file"" flag is stored in ScriptOrigin.
// Step 21.4-21.5. // Step 24.4-24.5.
let url = match base_url.join(&src) { let url = match base_url.join(&src) {
Ok(url) => url, Ok(url) => url,
Err(_) => { Err(_) => {
@ -474,64 +493,89 @@ impl HTMLScriptElement {
}, },
}; };
// Preparation for step 23. match script_type {
let kind = if element.has_attribute(&local_name!("defer")) && ScriptType::Classic => {
was_parser_inserted && // Preparation for step 26.
!r#async let kind = if element.has_attribute(&local_name!("defer")) &&
{ was_parser_inserted &&
// Step 23.a: classic, has src, has defer, was parser-inserted, is not async. !r#async
ExternalScriptKind::Deferred {
} else if was_parser_inserted && !r#async { // Step 26.a: classic, has src, has defer, was parser-inserted, is not async.
// Step 23.c: classic, has src, was parser-inserted, is not async. ExternalScriptKind::Deferred
ExternalScriptKind::ParsingBlocking } else if was_parser_inserted && !r#async {
} else if !r#async && !self.non_blocking.get() { // Step 26.c: classic, has src, was parser-inserted, is not async.
// Step 23.d: classic, has src, is not async, is not non-blocking. ExternalScriptKind::ParsingBlocking
ExternalScriptKind::AsapInOrder } else if !r#async && !self.non_blocking.get() {
} else { // Step 26.d: classic, has src, is not async, is not non-blocking.
// Step 23.f: classic, has src. ExternalScriptKind::AsapInOrder
ExternalScriptKind::Asap } else {
}; // Step 26.f: classic, has src.
ExternalScriptKind::Asap
};
// Step 21.6. // Step 24.6.
fetch_a_classic_script( fetch_a_classic_script(
self, self,
kind, kind,
url, url,
cors_setting, cors_setting,
integrity_metadata.to_owned(), integrity_metadata.to_owned(),
encoding, encoding,
); );
// Step 23. // Step 23.
match kind { match kind {
ExternalScriptKind::Deferred => doc.add_deferred_script(self), ExternalScriptKind::Deferred => doc.add_deferred_script(self),
ExternalScriptKind::ParsingBlocking => { ExternalScriptKind::ParsingBlocking => {
doc.set_pending_parsing_blocking_script(self, None) doc.set_pending_parsing_blocking_script(self, None)
},
ExternalScriptKind::AsapInOrder => doc.push_asap_in_order_script(self),
ExternalScriptKind::Asap => doc.add_asap_script(self),
}
},
ScriptType::Module => {
warn!(
"{} is a module script. It should be fixed after #23545 landed.",
url.clone()
);
}, },
ExternalScriptKind::AsapInOrder => doc.push_asap_in_order_script(self),
ExternalScriptKind::Asap => doc.add_asap_script(self),
} }
} else { } else {
// Step 22. // Step 25.
assert!(!text.is_empty()); assert!(!text.is_empty());
let result = Ok(ClassicScript::internal(text, base_url));
// Step 23. // Step 25-1.
let result = Ok(ScriptOrigin::internal(
text.clone(),
base_url.clone(),
script_type.clone(),
));
// TODO: Step 25-2.
if let ScriptType::Module = script_type {
warn!(
"{} is a module script. It should be fixed after #23545 landed.",
base_url.clone()
);
return;
}
// Step 26.
if was_parser_inserted && if was_parser_inserted &&
doc.get_current_parser() doc.get_current_parser()
.map_or(false, |parser| parser.script_nesting_level() <= 1) && .map_or(false, |parser| parser.script_nesting_level() <= 1) &&
doc.get_script_blocking_stylesheets_count() > 0 doc.get_script_blocking_stylesheets_count() > 0
{ {
// Step 23.h: classic, has no src, was parser-inserted, is blocked on stylesheet. // Step 26.h: classic, has no src, was parser-inserted, is blocked on stylesheet.
doc.set_pending_parsing_blocking_script(self, Some(result)); doc.set_pending_parsing_blocking_script(self, Some(result));
} else { } else {
// Step 23.i: otherwise. // Step 26.i: otherwise.
self.execute(result); self.execute(result);
} }
} }
} }
fn unminify_js(&self, script: &mut ClassicScript) { fn unminify_js(&self, script: &mut ScriptOrigin) {
if !self.parser_document.window().unminify_js() { if !self.parser_document.window().unminify_js() {
return; return;
} }
@ -584,7 +628,7 @@ impl HTMLScriptElement {
} }
/// <https://html.spec.whatwg.org/multipage/#execute-the-script-block> /// <https://html.spec.whatwg.org/multipage/#execute-the-script-block>
pub fn execute(&self, result: Result<ClassicScript, NetworkError>) { pub fn execute(&self, result: Result<ScriptOrigin, NetworkError>) {
// Step 1. // Step 1.
let doc = document_from_node(self); let doc = document_from_node(self);
if self.parser_inserted.get() && &*doc != &*self.parser_document { if self.parser_inserted.get() && &*doc != &*self.parser_document {
@ -639,7 +683,7 @@ impl HTMLScriptElement {
} }
// https://html.spec.whatwg.org/multipage/#run-a-classic-script // https://html.spec.whatwg.org/multipage/#run-a-classic-script
pub fn run_a_classic_script(&self, script: &ClassicScript) { pub fn run_a_classic_script(&self, script: &ScriptOrigin) {
// TODO use a settings object rather than this element's document/window // TODO use a settings object rather than this element's document/window
// Step 2 // Step 2
let document = document_from_node(self); let document = document_from_node(self);
@ -688,45 +732,58 @@ impl HTMLScriptElement {
); );
} }
pub fn is_javascript(&self) -> bool { // https://html.spec.whatwg.org/multipage/#prepare-a-script Step 7.
pub fn get_script_type(&self) -> Option<ScriptType> {
let element = self.upcast::<Element>(); let element = self.upcast::<Element>();
let type_attr = element.get_attribute(&ns!(), &local_name!("type")); let type_attr = element.get_attribute(&ns!(), &local_name!("type"));
let is_js = match type_attr.as_ref().map(|s| s.value()) { let language_attr = element.get_attribute(&ns!(), &local_name!("language"));
Some(ref s) if s.is_empty() => {
// type attr exists, but empty means js let script_type = match (
type_attr.as_ref().map(|t| t.value()),
language_attr.as_ref().map(|l| l.value()),
) {
(Some(ref ty), _) if ty.is_empty() => {
debug!("script type empty, inferring js"); debug!("script type empty, inferring js");
true Some(ScriptType::Classic)
}, },
Some(s) => { (None, Some(ref lang)) if lang.is_empty() => {
debug!("script type={}", &**s); debug!("script type empty, inferring js");
SCRIPT_JS_MIMES Some(ScriptType::Classic)
.contains(&s.to_ascii_lowercase().trim_matches(HTML_SPACE_CHARACTERS))
}, },
None => { (None, None) => {
debug!("no script type"); debug!("script type empty, inferring js");
let language_attr = element.get_attribute(&ns!(), &local_name!("language")); Some(ScriptType::Classic)
let is_js = match language_attr.as_ref().map(|s| s.value()) { },
Some(ref s) if s.is_empty() => { (None, Some(ref lang)) => {
debug!("script language empty, inferring js"); debug!("script language={}", &***lang);
true let language = format!("text/{}", &***lang);
},
Some(s) => { if SCRIPT_JS_MIMES.contains(&language.to_ascii_lowercase().as_str()) {
debug!("script language={}", &**s); Some(ScriptType::Classic)
let mut language = format!("text/{}", &**s); } else {
language.make_ascii_lowercase(); None
SCRIPT_JS_MIMES.contains(&&*language) }
}, },
None => { (Some(ref ty), _) => {
debug!("no script type or language, inferring js"); debug!("script type={}", &***ty);
true
}, if &***ty == String::from("module") {
}; return Some(ScriptType::Module);
// https://github.com/rust-lang/rust/issues/21114 }
is_js
if SCRIPT_JS_MIMES
.contains(&ty.to_ascii_lowercase().trim_matches(HTML_SPACE_CHARACTERS))
{
Some(ScriptType::Classic)
} else {
None
}
}, },
}; };
// https://github.com/rust-lang/rust/issues/21114 // https://github.com/rust-lang/rust/issues/21114
is_js script_type
} }
pub fn set_parser_inserted(&self, parser_inserted: bool) { pub fn set_parser_inserted(&self, parser_inserted: bool) {