448862 - tinderstatus fails in current trunk - setting a property that has only a getter

Status: RESOLVED WONTFIX

(Firefox :: Extension Compatibility, defect)

Description

[Bill Gianopoulos [:WG9s]]

Something has changed which has resulted in gPrefService being defined when extensions are loaded.  This results in some extensions like tinderstatus failing to properly initialize.

The error is:

Error: setting a property that has only a getter
Source file: chrome://tinderstatus/content/tinderstatus.js
Line: 63

Line 63 of tinderstatus is:

var gPrefService = Cc["@mozilla.org/preferences-service;1"]
  .getService(Ci.nsIPrefService);


I am seeing a similar issue in an extension I wrote on this line:

var gPrefService = null;

Comment 1

[Bill Gianopoulos [:WG9s]]

I forgot to include the regression window.

Tinderstatus works fine with the 20080730 nightly and fails with the above error with the 20080731 nightly build.

Comment 2

[Bill Gianopoulos [:WG9s]]

Regression caused by check-in for bug 448572.

Confirmed via backout.

Comment 3

[Dão Gottwald [:dao]]

(In reply to comment #0)
> Error: setting a property that has only a getter
> Source file: chrome://tinderstatus/content/tinderstatus.js
> Line: 63
> 
> Line 63 of tinderstatus is:
> 
> var gPrefService = Cc["@mozilla.org/preferences-service;1"]
>   .getService(Ci.nsIPrefService);

Simple solution:

if (!gPrefService)
  gPrefService = ...;

> I am seeing a similar issue in an extension I wrote on this line:
> 
> var gPrefService = null;

This is pointless. Remove it.

Comment 4

[Dão Gottwald [:dao]]

Attachment: add a gPrefService setter

Comment 5

[Dão Gottwald [:dao]]

We could also just ignore setting a value:

__defineSetter__("gPrefService", function(val) val);

Comment 6

[Dão Gottwald [:dao]]

So either way the tinderstatus extension would fail because it sets and uses gPrefService as nsIPrefService rather than nsIPrefBranch2. The only way to solve this seems to be backing out that part of bug 448572, which I don't think we want.

Comment 7

[Bill Gianopoulos [:WG9s]]

Well then, shouldn't the name of the global be changed to gPrefBranch if it is not a nsIPrefService?

Comment 8

[Dão Gottwald [:dao]]

This would likely break other extensions.

Comment 9

[Bill Gianopoulos [:WG9s]]

(In reply to comment #6)
> So either way the tinderstatus extension would fail because it sets and uses
> gPrefService as nsIPrefService rather than nsIPrefBranch2. The only way to
> solve this seems to be backing out that part of bug 448572, which I don't think
> we want.
> 

Actually, I just did a build with the patch attached to this bug and tinderstatus appears to work just fine ,including being able to change preferences.

Comment 10

[Dão Gottwald [:dao]]

I think this doesn't break the browser because SanitizeListener() calls gPrefService.QueryInterface(Ci.nsIPrefService), and I don't think we want to depend on that.

Comment 11

[Dão Gottwald [:dao]]

Err, this would be the case if it queried nsIPrefBranch2, so I actually don't know why things don't break.

Comment 12

[Bill Gianopoulos [:WG9s]]

So, the intended way to leave this bug is that the official sample extension for Mozilla products no longer works under Firefox?

Comment 13

[Bill Gianopoulos [:WG9s]]

Should have made this clearer by mentioning that this extension is the one that is used in the sample tutorial on www.mozilla.org/docs/tutorials/ as the example of how to write a Mozilla extension.

Comment 14

[Dave Townsend [:mossop]]

(In reply to comment #12)
> So, the intended way to leave this bug is that the official sample extension
> for Mozilla products no longer works under Firefox?

No that isn't right. However the way to fix it is to fix the extension, not Firefox. I do not know who maintains it though.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Myk, Mark: can you update Tinderstatus for Bill?

Comment 16

[Myk Melez [:myk] [@mykmelez]]

(In reply to comment #13)
> Should have made this clearer by mentioning that this extension is the one that
> is used in the sample tutorial on www.mozilla.org/docs/tutorials/ as the
> example of how to write a Mozilla extension.

That tutorial is fairly outdated and shouldn't be considered authoritative.  The tutorial on developer.mozilla.org is the current canonical tutorial <https://developer.mozilla.org/en/Building_an_Extension>.


(In reply to comment #14)
> ... the way to fix it is to fix the extension, not Firefox.

That's right.  It's a bug that tinderstatus sets global variables like gPrefService.  I fixed the specific problem with gPrefService on October 3 in changeset ddb2c347600a <http://hg.mozdev.org/tinderstatus/rev/ddb2c347600a>, and the fixed version is available from AMO <https://addons.mozilla.org/en-US/firefox/addon/832>.