Last Comment Bug 740710 - B2G SMS: Timezone offset sign is wrong
: B2G SMS: Timezone offset sign is wrong
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla14
Assigned To: Kan-Ru Chen [:kanru] (UTC+8)
:
:
Mentors:
: 743361 (view as bug list)
Depends on:
Blocks: b2g-sms
  Show dependency treegraph
 
Reported: 2012-03-29 23:41 PDT by Tim Guan-tin Chien [:timdream] (please needinfo)
Modified: 2012-04-09 10:14 PDT (History)
4 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
timezone offset sign is wrong (1.39 KB, patch)
2012-04-06 02:50 PDT, Kan-Ru Chen [:kanru] (UTC+8)
philipp: review+
Details | Diff | Splinter Review

Description Tim Guan-tin Chien [:timdream] (please needinfo) 2012-03-29 23:41:39 PDT
When trying to send an SMS and send back one, the timestamp shown in log is hours into the future. Not a Gaia issue.

I/Gecko ( 2614): SmsDatabaseService: Going to store {"delivery":"sent","sender":null,"receiver":"+886912345678","body":"Hello!","timestamp":1332999591630,"id":11} I/Gecko ( 2614): SmsDatabaseService: Going to store {"delivery":"received","sender":"+886912345678","receiver":null,"body":"Hello back!","timestamp":1333057268000,"id":12}

It has been reported people on different timezone sees different offsets.

Gaia issue: https://github.com/andreasgal/gaia/issues/1074#issuecomment-4797291
Comment 1 Philipp von Weitershausen [:philikon] 2012-03-30 09:45:40 PDT
It looks a lot like the timezone offset is added at least twice to the timestamp. I bet the RIL worker and/or the conversion of timestamp to Date object to timestamp to Date object is messing it up somewhere.
Comment 2 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-04-01 01:10:03 PDT
Yeah, good catch. the received message is 16 hrs and 76.37 seconds ahead of the sent message. 76.37 secs is reasonable typing time of mine during the manual test.
Comment 3 Kan-Ru Chen [:kanru] (UTC+8) 2012-04-01 18:44:17 PDT
I'd like to look into this.
Comment 4 Philipp von Weitershausen [:philikon] 2012-04-03 17:13:03 PDT
Go for it! Thanks!
Comment 5 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-04-04 02:24:16 PDT
\O/
Comment 6 Kan-Ru Chen [:kanru] (UTC+8) 2012-04-06 02:50:47 PDT
Created attachment 612843 [details] [diff] [review]
timezone offset sign is wrong

Need to rebase because bug 736697 touched here.
Comment 7 Philipp von Weitershausen [:philikon] 2012-04-06 17:24:11 PDT
*** Bug 743361 has been marked as a duplicate of this bug. ***
Comment 8 Philipp von Weitershausen [:philikon] 2012-04-06 17:28:04 PDT
Comment on attachment 612843 [details] [diff] [review]
timezone offset sign is wrong

>       // If the most significant bit of the least significant nibble is 1,
>       // the timezone offset is negative (fourth bit from the right => 0x08).
>+      //   localtime = UTC + tzOffset
>+      // so
>+      //   UTC = localtime - tzOffset

s/so/therefore/

Yeah, good catch! Apparently I suck at math :)

r=me. I can address the small change above and push this fix. Thanks!
Comment 9 Kan-Ru Chen [:kanru] (UTC+8) 2012-04-06 17:34:07 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #8)
> Comment on attachment 612843 [details] [diff] [review]
> timezone offset sign is wrong
> 
> >       // If the most significant bit of the least significant nibble is 1,
> >       // the timezone offset is negative (fourth bit from the right => 0x08).
> >+      //   localtime = UTC + tzOffset
> >+      // so
> >+      //   UTC = localtime - tzOffset
> 
> s/so/therefore/
> 
> Yeah, good catch! Apparently I suck at math :)
> 
> r=me. I can address the small change above and push this fix. Thanks!

Yes, push it! :)
Comment 10 Philipp von Weitershausen [:philikon] 2012-04-06 17:43:43 PDT
(In reply to Kan-Ru Chen [:kanru] from comment #6)
> Need to rebase because bug 736697 touched here.

Next time, please do this asap right after asking for review. That way it creates less churn when pushing the patch. Thanks!
Comment 11 Philipp von Weitershausen [:philikon] 2012-04-06 17:50:28 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7a4dfac3acf
Comment 12 Kan-Ru Chen [:kanru] (UTC+8) 2012-04-06 18:12:18 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #10)
> (In reply to Kan-Ru Chen [:kanru] from comment #6)
> > Need to rebase because bug 736697 touched here.
> 
> Next time, please do this asap right after asking for review. That way it
> creates less churn when pushing the patch. Thanks!

Ah.. I was meant to rebase when the changes hit m-c. Thanks for you super fast review :)
Comment 13 Philipp von Weitershausen [:philikon] 2012-04-06 18:15:41 PDT
(In reply to Kan-Ru Chen [:kanru] from comment #12)
> (In reply to Philipp von Weitershausen [:philikon] from comment #10)
> > (In reply to Kan-Ru Chen [:kanru] from comment #6)
> > > Need to rebase because bug 736697 touched here.
> > 
> > Next time, please do this asap right after asking for review. That way it
> > creates less churn when pushing the patch. Thanks!
> 
> Ah.. I was meant to rebase when the changes hit m-c.

Oh I see. They just hit m-c from inbound. That's fair.
Comment 14 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-04-07 12:53:04 PDT
@kanru, you are the man!
Comment 15 Matt Brubeck (:mbrubeck) 2012-04-09 10:14:16 PDT
https://hg.mozilla.org/mozilla-central/rev/d7a4dfac3acf

Note You need to log in before you can comment on or make changes to this bug.