Last Comment Bug 753542 - Add prefs to enable/disable E4X (javascript.options.xml.content and .chrome)
: Add prefs to enable/disable E4X (javascript.options.xml.content and .chrome)
Status: RESOLVED FIXED
[js:p2]
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Other Branch
: All All
: -- normal (vote)
: mozilla16
Assigned To: Jason Orendorff [:jorendorff]
:
Mentors:
Depends on: 753885
Blocks: ExactRooting
  Show dependency treegraph
 
Reported: 2012-05-09 14:22 PDT by Jason Orendorff [:jorendorff]
Modified: 2012-06-13 09:51 PDT (History)
5 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
WIP 1, part 1 - Rename JSOPTION_XML -> JSOPTION_MOAR_XML and other renaming (50.31 KB, patch)
2012-05-16 09:12 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Splinter Review
WIP 1, part 2 - JSOPTION_ALLOW_XML (25.13 KB, patch)
2012-05-16 09:13 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Splinter Review
WIP 1, part 3 - add prefs for E4X support (6.63 KB, patch)
2012-05-16 09:39 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Splinter Review
v2 (10.80 KB, patch)
2012-05-23 10:51 PDT, Jason Orendorff [:jorendorff]
no flags Details | Diff | Splinter Review
v3 - also check the pref in dom/workers (12.52 KB, patch)
2012-05-31 09:19 PDT, Jason Orendorff [:jorendorff]
benjamin: review+
jst: superreview+
Details | Diff | Splinter Review

Description Jason Orendorff [:jorendorff] 2012-05-09 14:22:30 PDT

    
Comment 1 Jason Orendorff [:jorendorff] 2012-05-16 09:12:12 PDT
Created attachment 624405 [details] [diff] [review]
WIP 1, part 1 - Rename JSOPTION_XML -> JSOPTION_MOAR_XML and other renaming

The problem is that JSOPTION_XML doesn't do what you might guess, so I'm splitting it into two separate options:

    JSOPTION_ALLOW_XML - if this is off, we won't allow E4X syntax
      no matter what your version number is
    JSOPTION_MOAR_XML - if ALLOW_XML is off, this has no effect;
      if ALLOW_XML is on, this has the effect of the existing JSOPTION_XML flag,
      which means we allow E4X even in JS version numbers that don't
      normally have it, and <!-- is treated as an E4X comment rather than
      as a // to-end-of-line comment.

This patch simply renames the existing JSOPTION_XML to JSOPTION_MOAR_XML,
and a few other trivial renamings/cleanups. There should be no change in behavior.
Comment 2 Jason Orendorff [:jorendorff] 2012-05-16 09:13:26 PDT
Created attachment 624406 [details] [diff] [review]
WIP 1, part 2 - JSOPTION_ALLOW_XML

Adds JSOPTION_ALLOW_XML and unconditionally turns it on throughout the browser (and JS test suites except jsapi-tests).
Comment 3 Jason Orendorff [:jorendorff] 2012-05-16 09:39:34 PDT
Created attachment 624419 [details] [diff] [review]
WIP 1, part 3 - add prefs for E4X support

This adds two prefs:
  javascript.options.xml.chrome
  javascript.options.xml.content

Both these prefs control JSOPTION_ALLOW_XML. The first is for chrome JS; the second is for web content.

In this patch, both default to true so as not to change the default behavior, but in a follow-up bug I intend to make them both off-by-default!

Note: In this WIP, the prefs only control XML syntax; neither pref affects the constructors XML, XMLList, Namespace, etc. that exist in every global. Those shouldn't exist either, if JSOPTION_ALLOW_XML is off. I just haven't implemented that part yet.
Comment 4 Jason Orendorff [:jorendorff] 2012-05-16 09:43:25 PDT
Oh - I should add that due to bugs in part 2, this isn't passing tests yet. At the moment, it's disabling XML support more than it should. Just putting the patches up early so anyone interested could look into the effect on addons.
Comment 5 Jason Orendorff [:jorendorff] 2012-05-22 17:29:22 PDT
https://tbpl.mozilla.org/?tree=Try&rev=34c01d5ee00e
Comment 6 Jason Orendorff [:jorendorff] 2012-05-23 10:51:28 PDT
Created attachment 626502 [details] [diff] [review]
v2
Comment 7 Jason Orendorff [:jorendorff] 2012-05-31 09:19:39 PDT
Created attachment 628773 [details] [diff] [review]
v3 - also check the pref in dom/workers

Sorry for the last-minute request. I'd like addon developers to be able to test their stuff using FF15 this release, and default the pref to false in FF16.
Comment 8 Jason Orendorff [:jorendorff] 2012-05-31 10:59:35 PDT
New Try run, since I added code:
  https://tbpl.mozilla.org/?tree=Try&rev=b96dd38052fa
Comment 9 Jason Orendorff [:jorendorff] 2012-06-06 19:56:01 PDT
Thank you for the rapid reviews!

https://hg.mozilla.org/integration/mozilla-inbound/rev/ce0c716baefd
Comment 10 Ed Morley [:emorley] 2012-06-07 05:46:09 PDT
https://hg.mozilla.org/mozilla-central/rev/ce0c716baefd

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