The default bug view has changed. See this FAQ.

Re-enable "for each" for scripts loaded from XUL/chrome://

RESOLVED FIXED in Firefox 20

Status

()

Core
JavaScript Engine
RESOLVED FIXED
4 years ago
4 years ago

People

(Reporter: Simon Kornblith, Assigned: emk)

Tracking

({addon-compat, regression})

20 Branch
mozilla21
x86
Mac OS X
addon-compat, regression
Points:
---
Bug Flags:
in-testsuite ?

Firefox Tracking Flags

(firefox19 unaffected, firefox20+ fixed, firefox21+ fixed)

Details

Attachments

(2 attachments)

(Reporter)

Description

4 years ago
Bug 804834 hid "for each" from content, but also seems to have had the unintended side effect of hiding it from XUL pages served from chrome:// unless type="application/javascript;version=1.8" is specified. This will presumably break a lot of extensions. Since bug 791343 was WONTFIXed, I assume this was not the intention.
(Reporter)

Comment 1

4 years ago
Created attachment 702188 [details]
XPI demonstrating issue

I've attached an XPI that demonstrates this issue. If you load chrome://bug830665/content/test.xul, the following error is logged to the console:

Timestamp: 1/15/13 5:15:14 AM
Error: SyntaxError: missing ( after for
Source File: chrome://bug830665/content/test.js
Line: 1, Column: 4
Source Code:
for each(var a in {}) {}
(Reporter)

Updated

4 years ago
Attachment #702188 - Attachment mime type: application/octet-stream → application/x-xpinstall
Blocks: 804834
Status: UNCONFIRMED → NEW
tracking-firefox20: --- → ?
tracking-firefox21: --- → ?
Ever confirmed: true
Keywords: regression
(Assignee)

Comment 2

4 years ago
In XUL, JS version will be set to the latest if the type attribute is present at all. No version parameter required.
I missed the case when the attribute itself is absent...
I think JS version should be set to the latest by default, even if the type attribute is not present. It's counterintuitive that <script> is different from <script type="text/javascript">.
status-firefox19: --- → unaffected
status-firefox20: --- → affected
status-firefox21: --- → affected
tracking-firefox20: ? → +
tracking-firefox21: ? → +
Keywords: addon-compat
Masatoshi will you be able to take on this follow-up work?
(Assignee)

Comment 4

4 years ago
Are there any add-on actually affected by this?
Jorge, maybe you can provide data in response to comment 4?
There are thousands of hits in the add-ons MXR, so this is pretty big. While the fix is relatively simple, this will break many add-ons, so I think we should solve this on the platform side.
Has this ever worked from XUL <script> nodes without the type attribute? Most JS 1.8 features, such as iterator/generator statements have not.
(Assignee)

Comment 8

4 years ago
(In reply to Jorge Villalobos [:jorgev] from comment #6)
> There are thousands of hits in the add-ons MXR, so this is pretty big. While
> the fix is relatively simple, this will break many add-ons, so I think we
> should solve this on the platform side.

Are those add-ons use a script element without type attribute? But OK, the fix is simple enough.

(In reply to Kris Maglione [:kmag] from comment #7)
> Has this ever worked from XUL <script> nodes without the type attribute?
> Most JS 1.8 features, such as iterator/generator statements have not.

Yes, for-each has worked in every JS versions.
(Assignee)

Comment 9

4 years ago
Created attachment 710298 [details] [diff] [review]
Use the latest JS version by default in XUL
Assignee: general → VYV03354
Status: NEW → ASSIGNED
Attachment #710298 - Flags: review?(jonas)
(In reply to Masatoshi Kimura [:emk] from comment #9)
> Created attachment 710298 [details] [diff] [review]
> Use the latest JS version by default in XUL

I think this is useful for parity with Cu.import and other methods of chrome script loading regardless of its impact on "for each".
(Assignee)

Comment 11

4 years ago
Hm, then I'd rather fix bug 824289. That is, enable for-each in chrome regardless of the JS version, but disable it in content.
Can we do both? If I wasn't clear, I'm in favor of XUL <script> nodes defaulting to the latest JS version.
Attachment #710298 - Flags: review?(jonas) → review+
(Assignee)

Comment 13

4 years ago
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c82d38cb905
Flags: in-qa-testsuite?
(Assignee)

Updated

4 years ago
Flags: in-qa-testsuite? → in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/4c82d38cb905
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
status-firefox21: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
I think this needs uplift to 20 if it's going to be effective.
(Assignee)

Comment 16

4 years ago
Comment on attachment 710298 [details] [diff] [review]
Use the latest JS version by default in XUL

[Approval Request Comment]
Bug caused by (feature/regressing bug #): 804834
User impact if declined: Some add-ons may stop to work on Firefox 20.
Testing completed (on m-c, etc.): on m-c
Risk to taking this patch (and alternatives if risky): low
String or UUID changes made by this patch: none
Attachment #710298 - Flags: approval-mozilla-aurora?
Comment on attachment 710298 [details] [diff] [review]
Use the latest JS version by default in XUL

Approving to uplift to prevent add-ons breaking on FF 20.
Attachment #710298 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 18

4 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/1a8298dfa8bf
status-firefox20: affected → fixed
You need to log in before you can comment on or make changes to this bug.