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

RESOLVED WONTFIX

Status

()

Firefox
Extension Compatibility
--
minor
RESOLVED WONTFIX
10 years ago
9 years ago

People

(Reporter: WG9s, Unassigned)

Tracking

({regression})

Trunk
regression
Points:
---
Bug Flags:
blocking-firefox3.5 -

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 obsolete attachment)

(Reporter)

Description

10 years ago
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;
Flags: blocking-firefox3.1?
(Reporter)

Comment 1

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

Updated

10 years ago
Summary: tinderstatus and other extensions don't work in current trunk builds → tinderstatus fails in current trunk - setting a property that has only a getter
(Reporter)

Comment 2

10 years ago
Regression caused by check-in for bug 448572.

Confirmed via backout.
Blocks: 448572
Keywords: regression

Comment 3

10 years ago
(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.

Updated

10 years ago
Component: General → Extension Compatibility
QA Contact: general → extension.compatibility

Updated

10 years ago
Severity: major → minor

Comment 4

10 years ago
Created attachment 332068 [details] [diff] [review]
add a gPrefService setter
Assignee: nobody → dao
Status: NEW → ASSIGNED
Attachment #332068 - Flags: review?(mano)

Comment 5

10 years ago
We could also just ignore setting a value:

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

Comment 6

10 years ago
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.
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → WONTFIX

Updated

10 years ago
Attachment #332068 - Attachment is obsolete: true
Attachment #332068 - Flags: review?(mano)
(Reporter)

Comment 7

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

Comment 8

10 years ago
This would likely break other extensions.
(Reporter)

Comment 9

10 years ago
(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.
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.
Err, this would be the case if it queried nsIPrefBranch2, so I actually don't know why things don't break.
(Reporter)

Comment 12

10 years ago
So, the intended way to leave this bug is that the official sample extension for Mozilla products no longer works under Firefox?
(Reporter)

Comment 13

10 years ago
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.
(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.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---

Updated

10 years ago
Assignee: dao → nobody
Status: REOPENED → NEW
Myk, Mark: can you update Tinderstatus for Bill?
Status: NEW → RESOLVED
Last Resolved: 10 years ago9 years ago
Flags: blocking-firefox3.1? → blocking-firefox3.1-
Resolution: --- → WONTFIX
(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>.
You need to log in before you can comment on or make changes to this bug.