Closed Bug 1054818 Opened 7 years ago Closed 4 years ago

[Sora][Call][Settings]The call time display abnormal after user change the system time.

Categories

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

defect
Not set
normal

Tracking

(b2g-v1.3 affected, b2g-v2.2 affected, b2g-master affected)

RESOLVED WONTFIX
Tracking Status
b2g-v1.3 --- affected
b2g-v2.2 --- affected
b2g-master --- affected

People

(Reporter: sync-1, Unassigned)

References

Details

(Whiteboard: [planned-sprint c=?])

Attachments

(7 files, 1 obsolete file)

Firefox OS v1.3
 AU_LINUX_GECKO_B2G_JB_3.2.01.03.00.112.301
 Mozilla build ID:20140422024003
 
 DEFECT DESCRIPTION:
 The call time display abnormal after user change the system time.
 
 REPRODUCING PROCEDURES:
 1.Establish a call(10086)
 2.Tap home key back to idle screen
 3.Launch "Settings" app -> Data&Time
 4.Change the system time 
 5.The call time also be changed(KO)
 
 EXPECTED BEHAVIOUR:
 KO:The call time should not be changed.
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 
 REPRODUCING RATE:
 100%
 
 For FT PR, Please list reference mobile's behavior:
Attached file jrdlog
Attached image Screenshot
Attached file log
Vance,

This bug also happens in V2.1, could you please help to check it?
Thanks.
Flags: needinfo?(vchen)
Hi Etienne -

Could you help to take a look at this one? it happens on master as well. if the user changes the time/date setting while in-call, the call duration time will change as well.

Thanks

Vance
Flags: needinfo?(vchen) → needinfo?(etienne)
Redirecting since I didn't follow the new developments around system time.
Flags: needinfo?(etienne) → needinfo?(drs+bugzilla)
I don't know much about this either, but I did some digging and it seems that the MozTimeManager web API doesn't provide an event for the system time changing. I wrote up a quick Gecko patch to add this, but then I looked further and found that there's an "onmoztimechange" event fired on the window element. This doesn't have blocking/feature status though, so I'm not going to get to this until after FL.
Whiteboard: [dialer-most-wanted]
I am adding qawanted to check if this is a regression in 1.3. Also interested in knowing if this has always been a problem or did we write something in 1.3 that this fallout is caused.
Keywords: qawanted
This is not a regression. The duration time comes from (Date.now() - startTime), so if changing the system time/date, Date.now is changing with it. The duration time is changed.
This bug does occur in 1.3 Flame. Adjusting the system time while in a call will also adjust the in call duration time.

***NOTE: I also found that adjusting the hours, kicks the user out of the time selector settings back to the call screen. This was rather unexpected.

Environmental Variables:
Device: Flame 1.3
Build ID: 20140627162151
Gaia: 5c43d012a9351e8aaeacda0b87b0886b7f67d33d
Gecko: e181a36ebafaa24e5390db9f597313406edfc794
Version: 28.0 (1.3)
Firmware Version: v123
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Keywords: qawanted
QA Contact: croesch
Qa-Wanted triage analysis: Not a regression, not a common user path / action
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
If someone wants to take this, here's my plan for it:

* Currently, we pass the start time into setInterval() and hold onto this for the duration of the call. [1]
* We should instead store the start time on the DOM of the duration element, perhaps as data-start-time. [2]
* We will also need to store the unlocalized duration, perhaps as data-duration. This should be updated every time the duration element itself is. [2]
* We then hook the "moztimechange" event. In here, we iterate over every duration element, and calculate its start time backwards with: |data-start-time = Date.now() - data-duration|. [2]

Note that we must do this backwards calculation instead of storing just a duration. The reason being that setTimeout() is unreliable and may not be called every second as requested. Indeed, the time it's actually called at usually has a significant epsilon. So by basing our calculations on a start time, we're never accumulating an error.

[1] https://github.com/mozilla-b2g/gaia/blob/master/apps/callscreen/js/call_screen.js#L531
[2] This should be unit tested.
Flags: needinfo?(drs+bugzilla)
Whiteboard: [dialer-most-wanted]
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Assignee: nobody → david.garciaparedes
Status: NEW → ASSIGNED
Attached file patch (obsolete) —
Attachment #8487982 - Flags: review?(drs+bugzilla)
Comment on attachment 8487982 [details] [review]
patch

This is good. I left some comments on the PR that are mostly related to commenting.
Attachment #8487982 - Flags: review?(drs+bugzilla) → review-
Dears,

This patch doesnot take the following case in account:
1.Outgoing a call to B, A-B ; And outgoing another call to C,A-C;
2.Merge these two call to a conference call A-BC.
3.Change the system time during the call, the duration for the conference call is right;
4.Hang up A-B, then B left the conferenc call, the call display is update, but the duration for A-C is still not right.

In function timechangeHandler(), var durationNodes = this.calls.querySelectorAll('.duration'); When there is a conference call, there is no '.duration' for every seperated calls. So the duration for these calls is still not update.
Whiteboard: [planned-sprint]
Target Milestone: --- → 2.1 S5 (26sep)
Gabriele suggested using https://developer.mozilla.org/en-US/docs/Web/API/Performance.now instead of the method described in comment 12. It should be as simple as replacing Date.now with Performance.now instead of all the DOM goop.

We should also file a followup to address the lack of the previous and new times in the moztimechange event.
David mentioned that Performance.now() doesn't work properly with the sinon.clock API. The attached patch stubs Performance.now() to use Date.now() instead, and should work for this purpose. The one thing I don't like is that we lose the precision of the Performance API, since it goes to msec but Date.now() only goes to sec.
Attached file patch v2
New patch using Performance.now.
I tried the mock, but I was not able to test the time change part because the mock is relaying on Date.now.
I'm stubbing performance.now with this:

  this.sinon.stub(performance, 'now', function() {
    return this.sinon.clock.now.toFixed(3);
  }.bind(this));
Attachment #8487982 - Attachment is obsolete: true
Attachment #8490042 - Flags: review?(drs+bugzilla)
Comment on attachment 8490042 [details] [review]
patch v2

Thanks for making the extra effort to add a test that makes sure that system time changes don't affect the duration. In this case, I don't think it makes sense to test that, since it's a property of the differences between the Performance and Date APIs. I left a couple of comments but this is otherwise great, and I'm happy that we found this simpler method.
Attachment #8490042 - Flags: review?(drs+bugzilla) → review+
Merged in master (David has not permission in Gaia to merge pr's):

https://github.com/mozilla-b2g/gaia/commit/7603a4ee080f371a79603fdf106aaa5287ff4437
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Duplicate of this bug: 952698
QA Contact: croesch
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Reverted on v2.2: https://github.com/mozilla-b2g/gaia/commit/79a416ce27b776733e705713d6121d68ed1149b3
Reverted on master: https://github.com/mozilla-b2g/gaia/commit/205da96f3edc6afee5e292c6c4bf7c40b092bfad
Assignee: david.garciaparedes → nobody
Priority: P2 → --
Whiteboard: [planned-sprint] → [planned-sprint c=?]
Target Milestone: 2.1 S5 (26sep) → ---
See Also: → 1150766
link all Fire C (codename: Sora) bugs to a meta one.
Firefox OS is not being worked on
Status: REOPENED → RESOLVED
Closed: 7 years ago4 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.