B2G SMS: Timezone offset sign is wrong

RESOLVED FIXED in mozilla14

Status

()

Core
DOM: Device Interfaces
RESOLVED FIXED
6 years ago
5 years ago

People

(Reporter: timdream, Assigned: kanru)

Tracking

(Blocks: 1 bug)

Trunk
mozilla14
ARM
Gonk (Firefox OS)
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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
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.
Blocks: 709564
Summary: Received SMS has timestamp offsets hours into the future → B2G SMS: Timezone offset is added to timestamp at least twice
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.
(Assignee)

Comment 3

6 years ago
I'd like to look into this.
Go for it! Thanks!
Assignee: nobody → kchen
\O/
(Assignee)

Comment 6

5 years ago
Created attachment 612843 [details] [diff] [review]
timezone offset sign is wrong

Need to rebase because bug 736697 touched here.
Attachment #612843 - Flags: review?(philipp)
Duplicate of this bug: 743361
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!
Attachment #612843 - Flags: review?(philipp) → review+
(Assignee)

Comment 9

5 years ago
(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! :)
(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!
Summary: B2G SMS: Timezone offset is added to timestamp at least twice → B2G SMS: Timezone offset sign is wrong
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7a4dfac3acf
(Assignee)

Comment 12

5 years ago
(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 :)
(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.
@kanru, you are the man!
https://hg.mozilla.org/mozilla-central/rev/d7a4dfac3acf
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla14
You need to log in before you can comment on or make changes to this bug.