Turn javascript.options.xml.chrome off by default

RESOLVED FIXED in mozilla20

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: gkw, Assigned: jorendorff)

Tracking

({addon-compat, dev-doc-complete})

Trunk
mozilla20
addon-compat, dev-doc-complete
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

(Reporter)

Description

5 years ago
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.
(Reporter)

Updated

5 years ago
Whiteboard: [jsbugmon:update]
(Assignee)

Updated

5 years ago
Assignee: general → jorendorff
(Reporter)

Updated

5 years ago
Blocks: 788293

Updated

5 years ago
Depends on: 791343
(Assignee)

Comment 1

5 years ago
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.
(Assignee)

Comment 2

5 years ago
Created attachment 688975 [details] [diff] [review]
Part 1 - Rewrite some XUL template tests to use DOM rather than E4X - v1
Attachment #688975 - Flags: review?(enndeakin)
(Assignee)

Comment 3

5 years ago
Green. https://tbpl.mozilla.org/?tree=Try&rev=7a2786e4b763
(Assignee)

Comment 4

5 years ago
Created attachment 689177 [details] [diff] [review]
Part 2 - Flip the pref - v1

This will require some serious outreach to addon developers.
Attachment #689177 - Flags: review?(jwalden+bmo)
Attachment #688975 - Flags: review?(enndeakin) → review+
Comment on attachment 689177 [details] [diff] [review]
Part 2 - Flip the pref - v1

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

Word.
Attachment #689177 - Flags: review?(jwalden+bmo) → review+
(Reporter)

Comment 6

5 years ago
> This will require some serious outreach to addon developers.

I'm adding dev-doc-needed, which I hope is the right keyword.
Keywords: checkin-needed, dev-doc-needed
(In reply to Jason Orendorff [:jorendorff] from comment #4)
> This will require some serious outreach to addon developers.

CCing Jorge.
Keywords: addon-compat
(Reporter)

Comment 8

5 years ago
Thanks for helping to add that keyword, xtc4uall! I was intending to do so after Waldo mentioned it, but it apparently slipped my mind..
https://hg.mozilla.org/integration/mozilla-inbound/rev/ed76f1a61dce
https://hg.mozilla.org/integration/mozilla-inbound/rev/100a1ea225f3
Keywords: checkin-needed
Jorge, do you have someplace you're keeping track of addons that use e4x where problems should be reported?
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.
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
Depends on: 820237
https://hg.mozilla.org/mozilla-central/rev/ed76f1a61dce
https://hg.mozilla.org/mozilla-central/rev/100a1ea225f3
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla20
Depends on: 820922
(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...
(Reporter)

Comment 15

5 years ago
> (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.
Depends on: 830823
This has been documented in various places.
Keywords: dev-doc-needed → dev-doc-complete
You need to log in before you can comment on or make changes to this bug.