Closed Bug 1342806 Opened 3 years ago Closed 3 years ago

Port bug 1342144 to mail [Remove version parameter from the type attribute of script elements]

Categories

(Thunderbird :: General, defect)

defect
Not set

Tracking

(Not tracked)

RESOLVED FIXED
Thunderbird 54.0

People

(Reporter: Paenglab, Assigned: Paenglab)

Details

Attachments

(2 files, 1 obsolete file)

M-C did some version removing in html and xhtml files. We should follow.
Attached patch noJSVersion.patch (obsolete) — Splinter Review
I used this sed script in /mail: find . ! -wholename '*/.hg*' -type f \( -iname '*.html' -o -iname '*.xhtml' -o -iname '*.xul' -o -iname '*.js' \) -exec sed -i -e 's/\(\(text\|application\)\/javascript\);version=1.[0-9]/\1/g' {} \;

I haven't touched following files because they use CR/LF line endings and this would be changed to LF: mail/base/test/unit/head_mailbase_maildir.js, mail/components/cloudfile/content/emptySettings.xhtml, mail/test/resources/mozmill/mozmill/extension/content/jquery/jquery-ui-1.7.1.custom.min.js and mail/test/resources/mozmill/mozmill/extension/content/overlay.js.
Assignee: nobody → richard.marti
Status: NEW → ASSIGNED
Attachment #8841372 - Flags: review?(acelists)
Thanks.

Can you also make a separate patch for the 3 cases in IM?

im/themes/messages/bubbles/Footer.html
<script type="application/javascript;version=1.8">
im/themes/messages/dark/Footer.html
<script type="application/javascript;version=1.8">
im/themes/messages/papersheets/Footer.html
<script type="application/javascript;version=1.8">

(In reply to Richard Marti (:Paenglab) from comment #1)
> I used this sed script in /mail: find . ! -wholename '*/.hg*' -type f \(
> -iname '*.html' -o -iname '*.xhtml' -o -iname '*.xul' -o -iname '*.js' \)
> -exec sed -i -e
> 's/\(\(text\|application\)\/javascript\);version=1.[0-9]/\1/g' {} \;
> 
> I haven't touched following files because they use CR/LF line endings and
> this would be changed to LF: mail/base/test/unit/head_mailbase_maildir.js,
> mail/components/cloudfile/content/emptySettings.xhtml,

I haven't found anything to change in these files.

> mail/test/resources/mozmill/mozmill/extension/content/jquery/jquery-ui-1.7.1.
> custom.min.js and
> mail/test/resources/mozmill/mozmill/extension/content/overlay.js.

This is fine, these are external files we should not change.
IM part.
Attachment #8841439 - Flags: review?(clokep)
(In reply to :aceman from comment #2)
> 
> (In reply to Richard Marti (:Paenglab) from comment #1)
> > I used this sed script in /mail: find . ! -wholename '*/.hg*' -type f \(
> > -iname '*.html' -o -iname '*.xhtml' -o -iname '*.xul' -o -iname '*.js' \)
> > -exec sed -i -e
> > 's/\(\(text\|application\)\/javascript\);version=1.[0-9]/\1/g' {} \;
> > 
> > I haven't touched following files because they use CR/LF line endings and
> > this would be changed to LF: mail/base/test/unit/head_mailbase_maildir.js,
> > mail/components/cloudfile/content/emptySettings.xhtml,
> 
> I haven't found anything to change in these files.

The script found them.
Comment on attachment 8841372 [details] [diff] [review]
noJSVersion.patch

Review of attachment 8841372 [details] [diff] [review]:
-----------------------------------------------------------------

::: mail/components/cloudfile/content/Box/management.xhtml
@@ +14,1 @@
>              src="chrome://messenger/content/protovis-r2.6-modded.js"/>

This seems to be a 3rd party library again so we will see if it works in the higher JS version. Sadly the cloudfile code is hard to test, if you don't have account at Box or Hightail.
Attachment #8841372 - Flags: review?(acelists) → review+
Comment on attachment 8841439 [details] [diff] [review]
noJSVersionIM.patch

Review of attachment 8841439 [details] [diff] [review]:
-----------------------------------------------------------------

Thanks!
Attachment #8841439 - Flags: review?(clokep) → review+
(In reply to :aceman from comment #5)
> Comment on attachment 8841372 [details] [diff] [review]
> noJSVersion.patch
> 
> Review of attachment 8841372 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: mail/components/cloudfile/content/Box/management.xhtml
> @@ +14,1 @@
> >              src="chrome://messenger/content/protovis-r2.6-modded.js"/>
> 
> This seems to be a 3rd party library again so we will see if it works in the
> higher JS version. Sadly the cloudfile code is hard to test, if you don't
> have account at Box or Hightail.

Should I revert this or is it okay to check-in like it is? I can't test too.
I'd say leave the version specified for total safety. The library will only get older and older and may stop working with the new JS features from m-c.
Reverted th removal of the version attribute for protovis-r2.6-modded.js.
Attachment #8841372 - Attachment is obsolete: true
Attachment #8841702 - Flags: review+
Keywords: checkin-needed
https://hg.mozilla.org/comm-central/rev/a4c510e01d6fd831ac80bcb936165ff1986e08a3
https://hg.mozilla.org/comm-central/rev/19fdf87ad6387611db4af35bb7fcf4b788044d09
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Keywords: checkin-needed
Resolution: --- → FIXED
Target Milestone: --- → Thunderbird 54.0
It seems there are still about 2 occurrences left (https://dxr.mozilla.org/comm-central/search?q=%3Cscript+type%3D%22application%2Fjavascript%3Bversion%3D&redirect=false), including suite. Would you do a followup?
You need to log in before you can comment on or make changes to this bug.