Last Comment Bug 788290 - Turn javascript.options.xml.chrome off by default
: Turn javascript.options.xml.chrome off by default
Status: RESOLVED FIXED
: addon-compat, dev-doc-complete
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: Trunk
: All All
: -- normal with 1 vote (vote)
: mozilla20
Assigned To: Jason Orendorff [:jorendorff]
: general
Mentors:
Depends on: 791343 820237 820922 830823
Blocks: 788293
  Show dependency treegraph
 
Reported: 2012-09-04 14:31 PDT by Gary Kwong [:gkw] [:nth10sd]
Modified: 2013-03-22 15:16 PDT (History)
22 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Part 1 - Rewrite some XUL template tests to use DOM rather than E4X - v1 (565.59 KB, patch)
2012-12-05 15:00 PST, Jason Orendorff [:jorendorff]
enndeakin: review+
Details | Diff | Splinter Review
Part 2 - Flip the pref - v1 (1.10 KB, patch)
2012-12-06 05:19 PST, Jason Orendorff [:jorendorff]
jwalden+bmo: review+
Details | Diff | Splinter Review

Description Gary Kwong [:gkw] [:nth10sd] 2012-09-04 14:31:34 PDT
As per https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine/yYQyMCcMf-0/discussion we should turn javascript.options.xml.chrome off by default on 18 nightly and 17 aurora.
Comment 1 Jason Orendorff [:jorendorff] 2012-09-14 16:14:51 PDT
https://tbpl.mozilla.org/?tree=Try&rev=ca47daf24169

Moth fails because every single test under content/xul/templates/tests/chrome is made of E4X. They have to be rewritten to put the XML (representing the expected output) in a separate file or something, and then templates_shared.js has to be rewritten to use DOM APIs instead of E4X. Not a ton of work, looks like.
Comment 2 Jason Orendorff [:jorendorff] 2012-12-05 15:00:55 PST
Created attachment 688975 [details] [diff] [review]
Part 1 - Rewrite some XUL template tests to use DOM rather than E4X - v1
Comment 3 Jason Orendorff [:jorendorff] 2012-12-06 05:17:52 PST
Green. https://tbpl.mozilla.org/?tree=Try&rev=7a2786e4b763
Comment 4 Jason Orendorff [:jorendorff] 2012-12-06 05:19:37 PST
Created attachment 689177 [details] [diff] [review]
Part 2 - Flip the pref - v1

This will require some serious outreach to addon developers.
Comment 5 Jeff Walden [:Waldo] (remove +bmo to email) 2012-12-07 15:33:52 PST
Comment on attachment 689177 [details] [diff] [review]
Part 2 - Flip the pref - v1

Review of attachment 689177 [details] [diff] [review]:
-----------------------------------------------------------------

Word.
Comment 6 Gary Kwong [:gkw] [:nth10sd] 2012-12-07 15:44:06 PST
> This will require some serious outreach to addon developers.

I'm adding dev-doc-needed, which I hope is the right keyword.
Comment 7 (mostly gone) XtC4UaLL [:xtc4uall] 2012-12-08 01:26:10 PST
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> This will require some serious outreach to addon developers.

CCing Jorge.
Comment 8 Gary Kwong [:gkw] [:nth10sd] 2012-12-08 04:26:20 PST
Thanks for helping to add that keyword, xtc4uall! I was intending to do so after Waldo mentioned it, but it apparently slipped my mind..
Comment 10 Boris Zbarsky [:bz] 2012-12-10 14:59:10 PST
Jorge, do you have someplace you're keeping track of addons that use e4x where problems should be reported?
Comment 11 Jorge Villalobos [:jorgev] 2012-12-10 16:33:37 PST
We have the Add-ons MXR. Kris was looking at the add-ons affected by this just now, so maybe he can share the query he used.
Comment 12 Kris Maglione [:kmag] 2012-12-10 16:48:33 PST
We should file Tech Evangelism > Add-on bugs for add-ons with a significant user base. [e4x-breakage] in the whiteboard should do for tracking.

There's really not a good MXR query for this. I'm currently doing some manual digging through [1] and finding more affected add-ons than I'd expected.

[1] https://mxr.mozilla.org/addons/search?string=%5BCDATA&case=1&find=%5C.js&findi=%5C.js&filter=%5E%5B%5E%5C0%5D*%24&hitlimit=&tree=addons
Comment 14 linux.user.since.2002 2012-12-16 17:54:38 PST
(In reply to Jorge Villalobos [:jorgev] from comment #11)
> We have the Add-ons MXR. Kris was looking at the add-ons affected by this
> just now, so maybe he can share the query he used.


The Addons MXR does not appear to be public, so users cannot confirm the query matches specific addons; however, the addon "Load Tabs Progressively" is affected.  Toggling the preference back to true makes it work as expected again.


(In reply to Gary Kwong [:gkw] from comment #6)
> > This will require some serious outreach to addon developers.
> 
> I'm adding dev-doc-needed, which I hope is the right keyword.


Had made an attempt to rewrite E4X section(s) of Load Tabs Progressively (I'm not the maintainer), but it must have been incomplete or incorrect as it threw additional errors and did not function as expected.  Setting javascript.options.xml.chrome = true for now (as this gets the addon to work as expected)...

Documentation would certainly be appreciated!


(In reply to Gary Kwong [:gkw] from comment #0)
> As per
> https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine/yYQyMCcMf-
> 0/discussion we should turn javascript.options.xml.chrome off by default on
> 18 nightly and 17 aurora.

This just landed a few days ago, yet the link indicates that it should be disabled by default in content, then chrome, then removed in successive versions all < 20.

Hope the documentation is generated before E4X is actually removed...
Comment 15 Gary Kwong [:gkw] [:nth10sd] 2012-12-16 21:46:48 PST
> (In reply to Gary Kwong [:gkw] from comment #0)
> > As per
> > https://groups.google.com/forum/#!topic/mozilla.dev.tech.js-engine/yYQyMCcMf-
> > 0/discussion we should turn javascript.options.xml.chrome off by default on
> > 18 nightly and 17 aurora.
> 
> This just landed a few days ago, yet the link indicates that it should be
> disabled by default in content, then chrome, then removed in successive
> versions all < 20.
> 
> Hope the documentation is generated before E4X is actually removed...

AFAIK, we're a little bit behind that schedule.
Comment 16 Tom Schuster [:evilpie] 2013-03-22 15:16:05 PDT
This has been documented in various places.

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