Closed Bug 830665 Opened 8 years ago Closed 7 years ago

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

Categories

(Core :: JavaScript Engine, defect)

20 Branch
x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla21
Tracking Status
firefox19 --- unaffected
firefox20 + fixed
firefox21 + fixed

People

(Reporter: simon, Assigned: emk)

References

Details

(Keywords: addon-compat, regression)

Attachments

(2 files)

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.
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 {}) {}
Attachment #702188 - Attachment mime type: application/octet-stream → application/x-xpinstall
Blocks: 804834
Status: UNCONFIRMED → NEW
Ever confirmed: true
Keywords: regression
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">.
Masatoshi will you be able to take on this follow-up work?
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.
(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: 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".
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.
Flags: in-qa-testsuite? → in-testsuite?
https://hg.mozilla.org/mozilla-central/rev/4c82d38cb905
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
I think this needs uplift to 20 if it's going to be effective.
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+
You need to log in before you can comment on or make changes to this bug.