Small improvements to nsScriptLoader.

RESOLVED WONTFIX

Status

()

Core
DOM
RESOLVED WONTFIX
6 years ago
5 years ago

People

(Reporter: evilpie, Assigned: evilpie)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

Comment hidden (empty)
(Assignee)

Comment 1

6 years ago
Created attachment 585292 [details] [diff] [review]
v1

I actually wanted to implement support for EcmaScript Harmony, but as this is not really needed, I decided to attach this patch anyways. The main improvement is that we show a warning if a script hasn't been executed, because the type is unknown.
Assignee: nobody → evilpies
Status: NEW → ASSIGNED
Attachment #585292 - Flags: review?(bzbarsky)
(In reply to Tom Schuster (evilpie) from comment #1)
> The main
> improvement is that we show a warning if a script hasn't been executed,
> because the type is unknown.

Note that HTML5 allows the use of non-JS script types for "data blocks" in the case of inline scripts. This is used e.g. for WebGL shaders. Do we want to warn about those?
Comment on attachment 585292 [details] [diff] [review]
v1

I think Henri is right.  Warning on unknown script types is not really desirable here....
Attachment #585292 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 4

6 years ago
Created attachment 624387 [details] [diff] [review]
just some small cleanup

So well ditched that part but kept some cleanup. Bug 738380 removed the way to detect the script type from the root element, but didn't update the commment.
Attachment #585292 - Attachment is obsolete: true
Attachment #624387 - Flags: review?(bzbarsky)
(Assignee)

Comment 5

6 years ago
Created attachment 624388 [details] [diff] [review]
forgot refresh
Attachment #624388 - Flags: review?(bzbarsky)
(Assignee)

Comment 6

6 years ago
Created attachment 624389 [details] [diff] [review]
Wow, I am stupid
Attachment #624387 - Attachment is obsolete: true
Attachment #624388 - Attachment is obsolete: true
Attachment #624387 - Flags: review?(bzbarsky)
Attachment #624388 - Flags: review?(bzbarsky)
Attachment #624389 - Flags: review?(bzbarsky)
Comment on attachment 624389 [details] [diff] [review]
Wow, I am stupid

>+++ b/content/base/src/nsScriptLoader.cpp
>   // XXX - still hard-coded for JS here, even though another language
>   // may be specified.  Should this check be made *after* we examine
>   // the attributes to locate the script-type?
>   // For now though, if JS is disabled we assume every language is
>   // disabled.

This part of the comment can go away, right?

>-  // XXX is this different from the mDocument->IsScriptEnabled() call?
>+  // This should not fail after we succeeded IsScriptEnabled()

Which "this"?  The old comment was talking about the context->GetScriptsEnabled() call below, though its placement was rather odd.

>@@ -620,17 +616,17 @@ nsScriptLoader::ProcessScriptElement(nsI
>-          "Non-XSLT Defer script on a document without an active parser; bug 592366.");
>+          "Non-XSLT Defer script on a document without an active parser; bug 592366."");

There's no way the new code here compiles.
Attachment #624389 - Flags: review?(bzbarsky) → review-
(Assignee)

Comment 8

5 years ago
This is not needed anymore.
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.