Closed Bug 878001 Opened 12 years ago Closed 11 years ago

Implement SNTP support (Gaia end)

Categories

(Firefox OS Graveyard :: Gaia::Settings, defect)

ARM
Gonk (Firefox OS)
defect
Not set
normal

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.
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.
I'm also interested on Gaia so I can take this too if you don't mind :)
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)
(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. :)
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.
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
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)
Redirected Comment 3 to firefoxos-ux-bugzilla
Flags: needinfo?(jcarpenter) → needinfo?(firefoxos-ux-bugzilla)
Ping UX team? We need UX feedback for the NITZ/SNTP settings.
(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)
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!
(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)
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)
Don't block on UX. Go ahead with a reasonable implementation, and we will adjust it once UX gives us feedback.
(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)
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)
Attached file Link to pull request 10334 (deleted) —
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)
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)
any feedback on the implementation?
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.
Neo, can you please provide your comments for UX part?
Flags: needinfo?(nhsieh)
Ping Neo? The Gaia implementation is currently blocked by the UX.
What kind of comment ? Can you explain more in detail ?
Flags: needinfo?(nhsieh) → needinfo?(kchang)
Neo, please check the attachments. Shao Hang want to change Timzone UX for NITZ.
Flags: needinfo?(kchang)
Hi Shao Hang Kao, Will system show alert window to let user knows this action (Through 3G/2G data) will spend their money ?
Not for current implementation, but the amount of transfer should be very small. Do we need to show it?
Flags: needinfo?(kyee)
Flags: needinfo?(nhsieh)
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: denehs → skao
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)
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 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-
Yes, you'll need to apply the patch in bug 874771 since some settings were renamed!
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.
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+)?
I’ll see that with Gene tomorrow, thanks for the quick reply.
Comment on attachment 761312 [details] Link to pull request 10334 fixed the nits, thanks
Attachment #761312 - Flags: review- → review?(kaze)
The Gecko part is good to go. I'd suggest let's land these two patches together to avoid confusion.
Comment on attachment 761312 [details] Link to pull request 10334 Works like a charm, thanks!
Attachment #761312 - Flags: review?(kaze) → review+
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Merged before branching.
blocking-b2g: koi? → ---
Attachment mime type: text/plain → text/x-github-pull-request
Flags: needinfo?(nhsieh)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: