Closed
Bug 878001
Opened 12 years ago
Closed 11 years ago
Implement SNTP support (Gaia end)
Categories
(Firefox OS Graveyard :: Gaia::Settings, defect)
Tracking
(b2g-v1.2 fixed)
RESOLVED
FIXED
Tracking | Status | |
---|---|---|
b2g-v1.2 | --- | fixed |
People
(Reporter: airpingu, Assigned: skao)
References
Details
Attachments
(4 files)
+++ This bug was initially created as a clone of Bug #874771 +++
Please see bug 874771, comment #31. To integrate the SNTP feature into our settings, we're sort of in agreement that changing the current time automatic-update related setting entries from
- time.nitz.available
- time.nitz.automatic-update.enabled
to
- time.clock.automatic-update.available
- time.clock.automatic-update.enabled
- time.timezone.automatic-update.available
- time.timezone.automatic-update.enabled
When the bug 874771 (Gecko end) lands, the Gaia end needs to have corresponding changes for the new settings. It'd be better to land the Gecko and Gaia parts at the same time as possible as we can to avoid compatibility issues.
Reporter | ||
Comment 1•12 years ago
|
||
Hi Kaze, are you interested in taking this on the Gaia end? Or could you please help assign this to someone else? Thanks!
Btw, I don't know who is the proper one to make the UX decision for the new settings.
Comment 2•12 years ago
|
||
I'm also interested on Gaia so I can take this too if you don't mind :)
Comment 3•12 years ago
|
||
This new setting to set the timezone automatically / manually should help simplifying the UI, so let’s get some UX input first.
Josh, we need some wireframes to integrate this new “automatic timezone selection” switch into the Settings and FTE apps — can we bother you with this?
Flags: needinfo?(jcarpenter)
Comment 4•12 years ago
|
||
(In reply to Shao Hang Kao from comment #2)
> I'm also interested on Gaia so I can take this too if you don't mind :)
I don’t mind as long as I’m in review. :)
Comment 5•12 years ago
|
||
Sorry I'm new to this system, how to assign this to myself? I only see a link on Assigned To and after clicking I'm sending mails to nobody@mozilla.org.
Comment 6•12 years ago
|
||
You should see a “take” link next to the “Assigned To” field, just click on it.
(I’ve just assigned you for this bug)
Assignee: nobody → denehs
Comment 7•12 years ago
|
||
Adding Eric since he's been working on time zone settings recently. Eric, can you please post links to the appropriate specs/folders in Box? I can see we have a lot there but am not sure which, if any, are ready for implementation.
Flags: needinfo?(epang)
Comment 8•12 years ago
|
||
Redirected Comment 3 to firefoxos-ux-bugzilla
Flags: needinfo?(jcarpenter) → needinfo?(firefoxos-ux-bugzilla)
Reporter | ||
Comment 9•12 years ago
|
||
Ping UX team? We need UX feedback for the NITZ/SNTP settings.
Comment 10•12 years ago
|
||
(In reply to Stephany Wilkes from comment #7)
> Adding Eric since he's been working on time zone settings recently. Eric,
> can you please post links to the appropriate specs/folders in Box? I can see
> we have a lot there but am not sure which, if any, are ready for
> implementation.
Hi Stephany, think you might be mistaken, I haven't worked on anything to do with
time zone settings :(.
If there was anything I would expect it to be in the FTE folder on Box:
https://mozilla.box.com/s/9iz7mctpyf3lchxjq768
To my knowledge this is new functionality and has not yet been spec'd out. Let me know if there's anything I can help with.
Flags: needinfo?(epang)
Comment 11•12 years ago
|
||
Hi Eric,
We're going to split the automatic set time setting into two:
1. automatic set time(system clock), where the source could be NITZ or SNTP (we choose it automatically internally)
2. automatic set timezone, which only available from NITZ now
Also I think we shoudn't hide the timezone field when automatic setting enabled, maybe gray out instead. So user could check if the timezone is correct.
Will UX have any spec for this or I can just start with a prototype?
Thanks!
Comment 12•12 years ago
|
||
(In reply to Shao Hang Kao from comment #11)
> Hi Eric,
>
> We're going to split the automatic set time setting into two:
> 1. automatic set time(system clock), where the source could be NITZ or
> SNTP (we choose it automatically internally)
> 2. automatic set timezone, which only available from NITZ now
>
> Also I think we shoudn't hide the timezone field when automatic setting
> enabled, maybe gray out instead. So user could check if the timezone is
> correct.
>
> Will UX have any spec for this or I can just start with a prototype?
>
> Thanks!
Hi Josh,
See above, maybe you can help us out :). Do you know if we have any specs for this? If you or someone can create some wireframes for this I don't mind putting together some visual designs for it (most likely next week).
Flags: needinfo?(jcarpenter)
Comment 13•12 years ago
|
||
Hi Kaze,
Since the gecko part was ready, is it reasonable for you that I upload a patch that just change the setting names (without change any UI behavior)? And after we got feedback from UX we'll have another patch for it.
Flags: needinfo?(kaze)
Comment 14•12 years ago
|
||
Don't block on UX. Go ahead with a reasonable implementation, and we will adjust it once UX gives us feedback.
Comment 15•12 years ago
|
||
(In reply to Shao Hang Kao from comment #13)
> Hi Kaze,
>
> Since the gecko part was ready, is it reasonable for you that I upload a
> patch that just change the setting names (without change any UI behavior)?
> And after we got feedback from UX we'll have another patch for it.
Sure, go ahead!
Flags: needinfo?(kaze)
Comment 16•12 years ago
|
||
Hi there-
UX is prioritizing blocking+ items (vs ?) so hence the delay in the response.
If this is a special case of needsinfo that requires UX input but is not blocking, I'll need to figure out another system to pick these up.
I'll also follow up with our team to see if Josh is the right person.
Thanks!
Jaime
Flags: needinfo?(jcarpenter) → needinfo?(swilkes)
Comment 17•12 years ago
|
||
Hello Kaze,
I implemented this way:
1. I assume the timezone we get from NITZ is always correct.
2. So no additional auto-timezone switch needed, instead the original one listen to both time.clock|timezone.automatic-update.available
3. When the check box is on we set both time.clock|timezone.automatic-update.enabled to true (and false when off)
4. When the check box is on, still shows date/time/timezone fields but disable them.
5. If only SNTP available (time.timezone.automatic-update.available = false), time zone fields won't get disabled.
I was trying to have some screen shots but can't get it work on my Inari, can't trigger home + power button in the same time. Is there another way to get screen shots remotely? I tried ddms tool from android sdk but just got a black screen.
Attachment #761312 -
Flags: review?(kaze)
Comment 18•12 years ago
|
||
Flagging Casey to review the implementation of timezone behavior. Once there is some UX feedback another patch can be spun up. Casey, please reassign as necessary or appropriate.
Flags: needinfo?(swilkes)
Flags: needinfo?(kyee)
Flags: needinfo?(firefoxos-ux-bugzilla)
Comment 19•11 years ago
|
||
any feedback on the implementation?
Comment 20•11 years ago
|
||
Comment 21•11 years ago
|
||
Comment 22•11 years ago
|
||
Comment 23•11 years ago
|
||
I just found the timezone we set from NITZ is a string like "UTC+08:00" instead of "Asia/Taipei", so I made a change to display the string directly instead of the region/city selections when nitz available. (code updated in the pull request)
Also uploaded some screen shots with different states (no source available, only sntp available and nitz available), sorry I still can't get screen shots directly from Inari so it's take by another phone with low quality.
Comment 24•11 years ago
|
||
Neo, can you please provide your comments for UX part?
Flags: needinfo?(nhsieh)
Reporter | ||
Comment 25•11 years ago
|
||
Ping Neo? The Gaia implementation is currently blocked by the UX.
Comment 26•11 years ago
|
||
What kind of comment ? Can you explain more in detail ?
Flags: needinfo?(nhsieh) → needinfo?(kchang)
Comment 27•11 years ago
|
||
Neo, please check the attachments. Shao Hang want to change Timzone UX for NITZ.
Flags: needinfo?(kchang)
Comment 28•11 years ago
|
||
Hi Shao Hang Kao, Will system show alert window to let user knows this action (Through 3G/2G data) will spend their money ?
Comment 29•11 years ago
|
||
Not for current implementation, but the amount of transfer should be very small. Do we need to show it?
Updated•11 years ago
|
Flags: needinfo?(kyee)
Updated•11 years ago
|
Flags: needinfo?(nhsieh)
Comment 30•11 years ago
|
||
Some quick UX feedback:
The implementation looks good. The only change that I would make is to remove the "Set manually" heading to avoid any confusion with the automatic setting.
Also, showing the UTC offset in place of the Region/City is fine as well.
Assignee | ||
Updated•11 years ago
|
Assignee: denehs → skao
Assignee | ||
Comment 31•11 years ago
|
||
Hi Kaze,
I've update the implementation per UX's suggestion (remove "Set manually" header). Could you review it? Thanks!
S.H.
Flags: needinfo?(kaze)
Comment 32•11 years ago
|
||
A few nitpicks, see comments on your PR. Please make sure your patch passes the linter.
Silly question: do I have to apply your patch in bug 874771 to make it work? Currently I can’t get any “Set automatically” switch, even with wifi connectivity.
Flags: needinfo?(kaze)
Comment 33•11 years ago
|
||
Comment on attachment 761312 [details]
Link to pull request 10334
Without your patch, I can see a “Set automatically” switch and use it to set the time from NITZ.
With your patch, I can’t use any “Set automatically” switch any more.
Could you please check that your patch doesn’t introduce any regression and ask for r? again?
Attachment #761312 -
Flags: review?(kaze) → review-
Assignee | ||
Comment 34•11 years ago
|
||
Yes, you'll need to apply the patch in bug 874771 since some settings were renamed!
Comment 35•11 years ago
|
||
I should have guessed. :-) I’ll rebuild Gecko with this patch tonight then.
It would be nice to support both Gecko versions (= before and after your patch). If it’s not possible (or not worth the trouble), I’d prefer to wait for the Gecko patch to land before r+’ing this one.
Assignee | ||
Comment 36•11 years ago
|
||
I see,
Gene told me we should land these two patch together. If we would like to support gecko without sntp support we'll have both old/new setting names in Gaia codes (at least in settings.js and datetime.js), which may confuse people? But if we land Gecko patch first without this patch lands the function won't work too (since we have new Gecko with old Gaia now). Is it possible to land these two patch in the same time (when both patches get r+)?
Comment 37•11 years ago
|
||
I’ll see that with Gene tomorrow, thanks for the quick reply.
Assignee | ||
Comment 38•11 years ago
|
||
Comment on attachment 761312 [details]
Link to pull request 10334
fixed the nits, thanks
Attachment #761312 -
Flags: review- → review?(kaze)
Reporter | ||
Comment 39•11 years ago
|
||
The Gecko part is good to go. I'd suggest let's land these two patches together to avoid confusion.
Comment 40•11 years ago
|
||
Comment on attachment 761312 [details]
Link to pull request 10334
Works like a charm, thanks!
Attachment #761312 -
Flags: review?(kaze) → review+
Comment 41•11 years ago
|
||
Merged on master: https://github.com/mozilla-b2g/gaia/commit/b64ef7c4b86f0d7f11918b1dafe6c96631f25811
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Comment 42•11 years ago
|
||
Merged before branching.
blocking-b2g: koi? → ---
status-b2g-v1.2:
--- → fixed
Updated•11 years ago
|
Attachment mime type: text/plain → text/x-github-pull-request
Updated•11 years ago
|
Flags: needinfo?(nhsieh)
Updated•9 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•