Last Comment Bug 381031 - make JS1.8 the default for <xul:script>
: make JS1.8 the default for <xul:script>
: dev-doc-complete
Product: Core
Classification: Components
Component: XUL (show other bugs)
: Trunk
: All All
-- normal (vote)
: mozilla1.9alpha6
Assigned To: Nickolay_Ponomarev
: Neil Deakin
: 381618 (view as bug list)
Depends on: 382182
Blocks: js1.8 385159
  Show dependency treegraph
Reported: 2007-05-17 05:25 PDT by Nickolay_Ponomarev
Modified: 2008-07-31 03:23 PDT (History)
6 users (show)
dbaron: blocking1.9-
asqueella: in‑testsuite?
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

patch (1.23 KB, patch)
2007-05-27 16:33 PDT, Nickolay_Ponomarev
no flags Details | Diff | Splinter Review
patch with JSVERSION_LATEST (6.55 KB, patch)
2007-06-07 14:50 PDT, Nickolay_Ponomarev
brendan: review+
brendan: superreview+
Details | Diff | Splinter Review
with the nit fixed (6.55 KB, patch)
2007-06-07 15:44 PDT, Nickolay_Ponomarev
no flags Details | Diff | Splinter Review

Description User image Nickolay_Ponomarev 2007-05-17 05:25:31 PDT
From bug 380236 comment 7:

Mike Shaver   2007-05-10 05:32:08 PDT

Related (I think): can/should we make JS1.8 the default for XUL-loaded JS in
Gecko 1.9?  We didn't in 1.8.1 for good compatibility reasons, but it makes for
a lot of poking around by extension and app authors to get the right versioning
parameters, and I think we'd do better by them in 1.9 to make the switch.
Comment 1 User image Nickolay_Ponomarev 2007-05-23 02:31:31 PDT
*** Bug 381618 has been marked as a duplicate of this bug. ***
Comment 2 User image Nickolay_Ponomarev 2007-05-27 16:33:19 PDT
Created attachment 266300 [details] [diff] [review]

(Depends on bug 382182.)

I dislike how it duplicates the version constant from the JS_SetVersion call here:

Would it be better to add a JSVERSION_LATEST value to the JSVersion enum?
Comment 3 User image Brendan Eich [:brendan] 2007-05-29 23:47:08 PDT
Yes, JSVERSION_LATEST -- just do it in this bug.

Comment 4 User image Brendan Eich [:brendan] 2007-06-04 09:33:30 PDT
Nickolay: ping? I can approve that patch but would rather see JSVERSION_LATEST, unless you prefer to do it in a separate bug.

Comment 5 User image Nickolay_Ponomarev 2007-06-04 10:11:38 PDT
Sorry, didn't have the time near my dev machine lately. I'll post an updated patch  tonight or tomorrow.
Comment 6 User image Nickolay_Ponomarev 2007-06-07 14:50:00 PDT
Created attachment 267630 [details] [diff] [review]

Better late than never!
Comment 7 User image Brendan Eich [:brendan] 2007-06-07 14:58:25 PDT
Comment on attachment 267630 [details] [diff] [review]

>             rv = mimeHdrParser->GetParameter(typeAndParams, "version",
>                                              EmptyCString(), PR_FALSE, nsnull,
>                                              versionName);
>             if (NS_FAILED(rv)) {
>+              // no version specified - version remains the default.
>+              if (rv != NS_ERROR_INVALID_ARG)
>+                return rv;

Since you fixed indentation here, could you also move the "no version specified ..." comment to after the if-early-return, which is exclusive of the comment's sense?

r+sr=me with that, thanks.

Comment 8 User image Nickolay_Ponomarev 2007-06-07 15:44:33 PDT
Created attachment 267642 [details] [diff] [review]
with the nit fixed
Comment 9 User image :Gavin Sharp [email:] 2007-06-07 15:50:44 PDT
mozilla/content/xul/document/src/nsXULContentSink.cpp 	1.183
mozilla/js/src/jspubtd.h 	3.84
mozilla/js/src/xpconnect/loader/mozJSComponentLoader.cpp 	1.136
mozilla/extensions/jssh/nsJSSh.cpp 	1.11
Comment 10 User image Nickolay_Ponomarev 2007-06-07 16:35:49 PDT
Brendan, thanks for the quick reviews.

Gavin, thanks for the checkin.
Comment 11 User image Nickolay_Ponomarev 2007-09-24 08:55:10 PDT
dev-doc-complete: Mentioned on

Note You need to log in before you can comment on or make changes to this bug.