Closed
Bug 830665
Opened 12 years ago
Closed 12 years ago
Re-enable "for each" for scripts loaded from XUL/chrome://
Categories
(Core :: JavaScript Engine, defect)
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)
(deleted),
application/x-xpinstall
|
Details | |
(deleted),
patch
|
sicking
:
review+
lsblakk
:
approval-mozilla-aurora+
|
Details | Diff | Splinter Review |
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•12 years ago
|
||
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•12 years ago
|
Attachment #702188 -
Attachment mime type: application/octet-stream → application/x-xpinstall
Updated•12 years ago
|
Blocks: 804834
Status: UNCONFIRMED → NEW
tracking-firefox20:
--- → ?
tracking-firefox21:
--- → ?
Ever confirmed: true
Keywords: regression
Assignee | ||
Comment 2•12 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">.
Updated•12 years ago
|
status-firefox19:
--- → unaffected
status-firefox20:
--- → affected
status-firefox21:
--- → affected
Updated•12 years ago
|
Keywords: addon-compat
Comment 3•12 years ago
|
||
Masatoshi will you be able to take on this follow-up work?
Assignee | ||
Comment 4•12 years ago
|
||
Are there any add-on actually affected by this?
Comment 6•12 years ago
|
||
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.
Comment 7•12 years ago
|
||
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•12 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•12 years ago
|
||
Comment 10•12 years ago
|
||
(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•12 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.
Comment 12•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/4c82d38cb905
Flags: in-qa-testsuite?
Assignee | ||
Updated•12 years ago
|
Flags: in-qa-testsuite? → in-testsuite?
Comment 14•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/4c82d38cb905
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla21
Comment 15•12 years ago
|
||
I think this needs uplift to 20 if it's going to be effective.
Assignee | ||
Comment 16•12 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 17•12 years ago
|
||
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•12 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/1a8298dfa8bf
You need to log in
before you can comment on or make changes to this bug.
Description
•