Closed
Bug 995206
Opened 10 years ago
Closed 10 years ago
Sync app.update.{channel,url} settings with preferences.
Categories
(Firefox OS Graveyard :: Runtime, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
1.4 S6 (25apr)
People
(Reporter: janx, Assigned: janx)
References
Details
Attachments
(1 file, 4 obsolete files)
4.92 KB,
patch
|
janx
:
review+
|
Details | Diff | Splinter Review |
To expose the app.update.{channel,url} preferences in Firefox OS' Developer menu, we need to create settings that are set to and update these preferences.
Assignee | ||
Comment 1•10 years ago
|
||
Updated•10 years ago
|
Component: Preferences → Gaia::Settings
Product: Firefox → Firefox OS
Version: Trunk → unspecified
Assignee | ||
Comment 2•10 years ago
|
||
Comment on attachment 8405406 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. Vivien, could you please have a look? Please complain if you feel this needs tests.
Attachment #8405406 -
Flags: review?(21)
Assignee | ||
Comment 3•10 years ago
|
||
Dão, this is a gecko change in the b2g/ folder, so I thought it wouldn't make sense to file it under Gaia::*. Could you please explain why you moved it there, so that in the future I'll know better where to file bugs?
Flags: needinfo?(dao)
Comment 4•10 years ago
|
||
You'll need to ask someone familiar with the b2g/ structure. All I know is b2g patches belong to the "Firefox OS" product, not "Firefox" (the desktop browser).
Component: Gaia::Settings → General
Flags: needinfo?(dao)
Updated•10 years ago
|
Component: General → Runtime
Comment 5•10 years ago
|
||
Comment on attachment 8405406 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. Review of attachment 8405406 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/settings.js @@ +564,5 @@ > + window.navigator.mozSettings.createLock().set({ > + 'deviceinfo.update_channel': value > + }); > + }); > +})(); Let's add a new flag to the simple mapping method at the end of the file. The flag would let you tell that the pref changes takes over settings changes.
Attachment #8405406 -
Flags: review?(21)
Assignee | ||
Comment 6•10 years ago
|
||
Attachment #8405406 -
Attachment is obsolete: true
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8408342 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. r=21 Review of attachment 8408342 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for the review! Here is a patch using a new `resetToPref` flag with the simple mapping mechanism.
Attachment #8408342 -
Flags: review?(21)
Comment 8•10 years ago
|
||
Comment on attachment 8408342 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. r=21 Review of attachment 8408342 [details] [diff] [review]: ----------------------------------------------------------------- ::: b2g/chrome/content/settings.js @@ +722,3 @@ > } > > + let setPref; nit: s/setPref/setPrefFunction
Attachment #8408342 -
Flags: review?(21) → review+
Assignee | ||
Comment 9•10 years ago
|
||
Attachment #8408342 -
Attachment is obsolete: true
Assignee | ||
Comment 10•10 years ago
|
||
Cheers for the review! (In reply to Vivien Nicolas (:vingtetun) (:21) from comment #8) > ::: b2g/chrome/content/settings.js > @@ +722,3 @@ > > } > > > > + let setPref; > > nit: s/setPref/setPrefFunction Added a comment instead to make sure people understand it's a function.
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8410143 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. r=21 21:r+
Attachment #8410143 -
Flags: review+
Assignee | ||
Comment 12•10 years ago
|
||
Try push just in case https://tbpl.mozilla.org/?tree=Try&rev=8690e4e02eb9
Assignee | ||
Comment 13•10 years ago
|
||
Apart from the few unrelated-looking fails, the try push seems green to me.
Keywords: checkin-needed
Comment 14•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/1ff975b6a290
Keywords: checkin-needed
Comment 15•10 years ago
|
||
I backed out since this patch is causing: E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] E/GeckoConsole(21949): [JavaScript Error: "TypeError: setPref is not a function" {file: "chrome://b2g/content/settings.js" line: 741}] during startup. https://hg.mozilla.org/integration/b2g-inbound/rev/ece426b71e9f
https://hg.mozilla.org/mozilla-central/rev/1ff975b6a290
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.4 S6 (25apr)
Assignee | ||
Comment 17•10 years ago
|
||
Wes, it looks like you cut b2g-inbound just after my patch, leaving Fabrice's backout of my patch behind. I'll investigate the `setPref` issue, but maybe you'll want the patch backed out on mozilla-central, either by merging b2g-inbound again or by backing it out separately?
Status: RESOLVED → REOPENED
Flags: needinfo?(kwierso)
Resolution: FIXED → ---
Assignee | ||
Comment 18•10 years ago
|
||
Removing the needinfo since you rolled b2g-inbound into https://hg.mozilla.org/mozilla-central/ again. I was able to fix my issue, but I'm waiting for http://git.mozilla.org/releases/gecko.git to update to mozilla-central so that I can rebase on top of the backout.
Flags: needinfo?(kwierso)
Assignee | ||
Comment 19•10 years ago
|
||
Attachment #8410143 -
Attachment is obsolete: true
Assignee | ||
Comment 20•10 years ago
|
||
Attachment #8411051 -
Attachment is obsolete: true
Assignee | ||
Comment 21•10 years ago
|
||
Comment on attachment 8411054 [details] [diff] [review] Sync app.update.{channel,url} settings with preferences. r=21 Thank you Fabrice for catching the logcat errors! The problem was due to: > let defaultValue = setting.defaultValue || setting; falling back to `setting` when `setting.defaultValue == false`, whereas the intention was to fall back to `setting` only when `setting.defaultValue === undefined`. Changed this line to: > let defaultValue = setting.defaultValue; > if (defaultValue === undefined) { > defaultValue = setting; > } Keeping Vivien's r+ since this is only a minor change to the patch.
Attachment #8411054 -
Flags: review+
Assignee | ||
Updated•10 years ago
|
Keywords: checkin-needed
Comment 22•10 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/1d7b9a01b8cf
Keywords: checkin-needed
Comment 23•10 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/1d7b9a01b8cf
Status: REOPENED → RESOLVED
Closed: 10 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•