1542877 - [meta] Stop using unnecessary type attributes (i.e. [type="application/javascript"]) on script tags

Status: NEW

(Core :: XUL, task, P3)

Description

[Brian Grinstead [:bgrins]]

This would help tighten up test boilerplate. It'll also make migration from xul->html documents easier (although AFAICT it doesn't hurt to have the attribute on html scripts, it's kind of weird).

Comment 1

[Boris Zbarsky [:bzbarsky]]

Looking at XULContentSinkImpl::OpenScript it seems like no type attr will cause isJavaScript to be true. That's basically the HTML behavior too: type defaults to "javascript" if no type specified.

So I'm not sure we even need any Gecko-side changes here, just changes to the <script> tags in XUL documents.

Comment 2

[Brian Grinstead [:bgrins]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #1)

Looking at XULContentSinkImpl::OpenScript it seems like no type attr will cause isJavaScript to be true. That's basically the HTML behavior too: type defaults to "javascript" if no type specified.

So I'm not sure we even need any Gecko-side changes here, just changes to the <script> tags in XUL documents.

Huh, that'll make things easier then. Working on a script to convert the frontend. By the way, is it the same story with type="text/javascript"?

Comment 3

[Boris Zbarsky [:bzbarsky]]

Yes. Basically, for our purposes there are only three different things you can stick in the type attr:

1. No attr at all: it's javascript
2. It's something in the list at https://searchfox.org/mozilla-central/rev/8d78f219702286c873860f39f9ed78bad1a6d062/dom/base/nsContentUtils.cpp#6994-7009: it's javascript
3. It's something else: not javascript, do not run.

Comment 4

[Brian Grinstead [:bgrins]]

From that list, I see hits for the following types in documents in the tree:

"text/javascript",
"text/ecmascript",
"application/javascript",
"application/ecmascript",
"application/x-javascript",

Comment 5

[Brian Grinstead [:bgrins]]

Attachment: find-replace-script-type.sh

First attempt at a script for generating this change

Comment 6

[Brian Grinstead [:bgrins]]

The patch is too large to upload to phab or splinter [10574 files changed, 26853 insertions(+), 28941 deletions(-)], but here it is: https://raw.githubusercontent.com/bgrins/firefox-patches/master/bug_1542877_scripted_change_to_replace_unnecessary_type_attributes_on_scrip.

Comment 7

[Boris Zbarsky [:bzbarsky]]

It's worth checking whether any of the test files being modified were testing the type attribute....

Comment 8

[Brian Grinstead [:bgrins]]

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #7)

It's worth checking whether any of the test files being modified were testing the type attribute....

Yeah, that makes sense. I can't think of a good way off-hand to assert that, but will think about it some more. I did exclude wpt from the script, under the assumption that there would be such tests there. In the meantime I've sent a notice to dev-platform and maybe someone on there will know of any tests that do this.

Comment 9

[Brian Grinstead [:bgrins]]

Attachment: find-replace-script-type.sh

Updated script that includes paths in ThirdPartyPaths.txt and .gitignore, and also allows for turning off particular directories. Also better chunking of args into the actual replacement command.

Comment 10

[Brian Grinstead [:bgrins]]

Attachment: Bug 1542877 - Part 1 - Update `./mach addtest` templates to stop using [type] attribute on script tags

Comment 11

[Brian Grinstead [:bgrins]]

Attachment: Bug 1542877 - Part 1 - Remove the [type] attribute for one-liner <script> tags loading files in chrome://mochikit/content/

This is an autogenerated commit to handle scripts loading mochitest harness files, in
the simple case where the script src is on the same line as the tag.

ignore-this-changeset

Comment 12

[Brian Grinstead [:bgrins]]

Attachment: find-replace-simpletest-js.sh

Part 2 was generated with this script

Comment 13

[Phabricator Automation]

Comment on attachment 9057091 [details]
Bug 1542877 - Part 1 - Update ./mach addtest templates to stop using [type] attribute on script tags

Revision D26811 was moved to bug 1543300. Setting attachment 9057091 [details] to obsolete.

Comment 14

[Phabricator Automation]

Comment on attachment 9057092 [details]
Bug 1542877 - Part 1 - Remove the [type] attribute for one-liner <script> tags loading files in chrome://mochikit/content/

Revision D26812 was moved to bug 1544322. Setting attachment 9057092 [details] to obsolete.