1090382 - JavaScript-navigator-property in .manifest causes silently breaks dom keyword access

Status: RESOLVED FIXED

(Core :: DOM: Core & HTML, defect)

Description

[Sam Penrose]

[Probably filed under incorrect component]
In bug 1062001, I implement a new dom keyword with a single attribute:

  dom.auth.authenticate()

The following .manifest file causes content to catch a "TypeError: navigator.auth.authenticate is not a function" Error when attempting to access dom.auth.authenticate(). Inspection reveals that it is undefined. There are no problems at build time. Deleting the JavaScript-navigator-property line allows the code to work. I believe this should either cause a build-time failure, or it should work. Please see the attachment to 1062001 for full context.
----------------------------
# nsDOMAuth.js
component {a073b455-faaf-4488-8e1e-45d3b4146d21} nsDOMAuth.js
contract @mozilla.org/dom/auth;1 {a073b455-faaf-4488-8e1e-45d3b4146d21}
category JavaScript-navigator-property auth @mozilla.org/dom/auth;1

# nsAuthService.js (initialization on startup)
component {50408e35-f647-45d5-9610-686e6482ab92} nsAuthService.js
contract @mozilla.org/dom/auth/service;1 50408e35-f647-45d5-9610-686e6482ab92
category app-startup AuthService @mozilla.org/dom/auth/service;1

Comment 1

[Josh Matthews [:jdm]]

For anybody following along at home, the problem is that Auth.webidl uses NavigatorProperty="auth", so the JavaScript-navigator-property is going to conflict with that. I have no idea how we would determine that at build time.

Comment 2

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

We could remove support for JavaScript-navigator-property.  We shouldn't be adding new code that uses that anyways.

Comment 3

[Josh Matthews [:jdm]]

Any chance addons use it?

Comment 4

[Kyle Huey (Exited; not receiving bugmail, old account, do not use)]

Almost certainly.

Comment 5

[Boris Zbarsky [:bzbarsky]]

The only way to deal at build time would be to have codegen spit out the list of names that are used with NavigatorProperty, have something in manifest processing spit out the list of names used with JavaScript-navigator-property, and have something compare the lists.

It's a lot of complexity to deal with this legacy mess; I prefer the suggestion in comment 2.  We have three non-test in-tree uses: mozPay, mozNetworkStats, and TCPSocket.  So we just need to fix bug 885982, bug 993311, and bug 1027734.  Which all have WIP patches, even.

> Almost certainly.

I looked on addons mxr.  If I ignore the fxos_1_2_simulator extension and the "Firefox OS simulator" extension, the only instances of the phrase "gator-prop":

1)  An extension named "Nulltxt" that provides navigator.bridge and navigator.crypto.  See <https://addons.mozilla.org/en-US/firefox/addon/nulltxt/>.  I will claim this has been superseded by webcrypto.

2)  Some extensions setting dom.navigator-property.disable.whatever prefs.

That's it.

I'm not sure what the story is with those simulator extensions; they're at https://mxr.mozilla.org/addons/source/410914/ and https://mxr.mozilla.org/addons/source/465236/

Comment 6

[Boris Zbarsky [:bzbarsky]]

JavaScript-navigator-property is gone, so this bug as filed is fixed, since it can't silently break anything.