Closed
Bug 772310
Opened 12 years ago
Closed 12 years ago
Marionette hits getBoolPref error on startup
Categories
(Remote Protocol :: Marionette, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
mozilla16
People
(Reporter: jruderman, Assigned: jgriffin)
References
Details
Attachments
(1 file)
1.93 KB,
patch
|
mdas
:
review+
|
Details | Diff | Splinter Review |
When starting a tbox debug build of Firefox on Mac (with a fresh profile), I get this error:
************************************************************
* Call to xpconnect wrapped JSObject produced this error: *
[Exception... "Component returned failure code: 0x8000ffff (NS_ERROR_UNEXPECTED) [nsIPrefBranch.getBoolPref]" nsresult: "0x8000ffff (NS_ERROR_UNEXPECTED)" location: "JS frame :: resource:///components/marionettecomponent.js :: mc_observe :: line 33" data: no]
************************************************************
> if (Services.prefs.getBoolPref('marionette.defaultPrefs.enabled')) {
Assignee | ||
Comment 1•12 years ago
|
||
See if the pref exists before we try to read from it.
Attachment #641247 -
Flags: review?(mdas)
Comment 2•12 years ago
|
||
Comment on attachment 641247 [details] [diff] [review] patch v0.1 >+const MARIONETTE_ENABLED_PREF = 'marionette.defaultPrefs.enabled'; [..] >- if (Services.prefs.getBoolPref('marionette.defaultPrefs.enabled')) { >+ if (Services.prefs.prefHasUserValue(MARIONETTE_ENABLED_PREF) && >+ Services.prefs.getBoolPref(MARIONETTE_ENABLED_PREF)) { Why are we making use of 'defaultPref'? We should use 'marionette.enabled' and set it as default in firefox.js. Whenever it is changed it will automatically has an user value. That way getBoolPref should never fail.
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #2) > Comment on attachment 641247 [details] [diff] [review] > patch v0.1 > > >+const MARIONETTE_ENABLED_PREF = 'marionette.defaultPrefs.enabled'; > [..] > >- if (Services.prefs.getBoolPref('marionette.defaultPrefs.enabled')) { > >+ if (Services.prefs.prefHasUserValue(MARIONETTE_ENABLED_PREF) && > >+ Services.prefs.getBoolPref(MARIONETTE_ENABLED_PREF)) { > > Why are we making use of 'defaultPref'? We should use 'marionette.enabled' > and set it as default in firefox.js. Whenever it is changed it will > automatically has an user value. That way getBoolPref should never fail. It probably should be just 'marionette.enabled', but that's a separate issue. Marionette is not included in release builds of Firefox, so I don't believe it makes sense to add a pref to firefox.js, that will never be accessed.
Comment 4•12 years ago
|
||
(In reply to Jonathan Griffin (:jgriffin) from comment #3) > It probably should be just 'marionette.enabled', but that's a separate issue. Has that already been filed? If not I can happily do so. > Marionette is not included in release builds of Firefox, so I don't believe > it makes sense to add a pref to firefox.js, that will never be accessed. I'm sure you can '#ifdef' that for whatever builds except releases. Also here I'm happy to file a bug on it if necessary.
Updated•12 years ago
|
Attachment #641247 -
Flags: review?(mdas) → review+
Comment 5•12 years ago
|
||
(In reply to Henrik Skupin (:whimboo) from comment #4) > (In reply to Jonathan Griffin (:jgriffin) from comment #3) > > It probably should be just 'marionette.enabled', but that's a separate issue. > > Has that already been filed? If not I can happily do so. Sure, I don't think we need defaultPrefs anymore, that was from back when we were trying out different security methods. > > > Marionette is not included in release builds of Firefox, so I don't believe > > it makes sense to add a pref to firefox.js, that will never be accessed. > > I'm sure you can '#ifdef' that for whatever builds except releases. Also > here I'm happy to file a bug on it if necessary. I don't think we want to dot hat since we don't want to set this value by default, it's something only used by test profiles, and in b2g.
Comment 6•12 years ago
|
||
(In reply to Malini Das [:mdas] from comment #5) > > I'm sure you can '#ifdef' that for whatever builds except releases. Also > > here I'm happy to file a bug on it if necessary. > > I don't think we want to dot hat since we don't want to set this value by > default, it's something only used by test profiles, and in b2g. Oh I see. That makes sense then.
Assignee | ||
Comment 7•12 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/70a11a81d2c5
Assignee: nobody → jgriffin
Target Milestone: --- → mozilla16
Comment 8•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/70a11a81d2c5
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Updated•1 year ago
|
Product: Testing → Remote Protocol
You need to log in
before you can comment on or make changes to this bug.
Description
•