Closed Bug 971617 Opened 10 years ago Closed 10 years ago

[Sora]Switching accounts,and select the folder to move the mail,the current account folder list display error.

Categories

(Firefox OS Graveyard :: Gaia::E-Mail, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED FIXED
1.4 S1 (14feb)

People

(Reporter: sync-1, Assigned: jrburke)

Details

(Keywords: regression, Whiteboard: [p=1])

Attachments

(2 files)

the latest id: Mozilla build ID: 20140118004001 FFOS: 1.3
 
 DEFECT DESCRIPTION:
 Switching accounts,and select the folder to move the mail,the current account folder list display error.
  
 REPRODUCING PROCEDURES:
  1,In idle interface ,open "Email" 
  2,Add two email accounts or more
  3,Switching from the current account to another account and do nothing
  4,Switch to the original account,and select a mail to move
  5,The folder list display error,It shows the previous account folder list
  6,Press cancel,and move a mail again ,the folder list is OK.
 
  EXPECTED BEHAVIOUR:
  It should always show the current account folder list.
 
  ASSOCIATE SPECIFICATION:
 
  TEST PLAN REFERENCE:
 
  TOOLS AND PLATFORMS USED:
 
  USER IMPACT:
 
  REPRODUCING RATE: 10/10
Does this reproduce on a 1.1 Buri build?
Flags: needinfo?(sync-1)
(In reply to comment #1)
 > Comment from Mozilla:Does this reproduce on a 1.1 Buri build?
 > 
 No,Cannot Reproduce on Buri 1.1,it is OK on 1.1 .
Flags: needinfo?(sync-1)
Keywords: regression
Can you include a video of the bug?
Flags: needinfo?(sync-1)
(In reply to Jason Smith [:jsmith] from comment #3)
> Can you include a video of the bug?

QA wanted for this.
Keywords: qawanted
(In reply to Jason Smith [:jsmith] from comment #3)
> Can you include a video of the bug?

Here you go : http://www.youtube.com/watch?v=7BVVg4ckkwU

Thanks

Vance
Flags: needinfo?(sync-1)
Created an attachment (id=634740)
 video of the bug
Created an attachment (id=634740)
 video of the bug
Attached video video of the bug
Created an attachment (id=634740)
 video of the bug
Keywords: qawanted
Attached file GitHub pull request
This occurred because model.js was using evt.emitWhenListener('foldersSlice') to emit events about a foldersSlice change. With two account changes right in a row, there were two events queued up for 'foldersSlice', the one for the incorrect one was first.

So when mail_common's ValueSelector use asked for model.latestOnce(), it got the first, out of date, event in the _pendingEvents queue.

On further inspection, model should not have been using emitWhenListener because it was only using that for generating events based on property IDs, and the `evt.latest*` methods check for property names on the object with that name already.

So the only concern would be for consumers of `model` that were using `model.on()` to get notified of a change and they register too late to receive that change. However, the `evt.latest*` methods were constructed specifically for the `model` uses where code just wants the latest value on the object, or get notified when it arrives.

This bore out when searching the email code for `model.on()`: the only cases where it is used care only about change events, not the "get current value or wait until there is one" cases where `latest` is used:

* mail_app calls `model.on('acctsSlice', ...)` and does so before model is inited, so gets all the change events.
 * message_list calls `model.on('newInboxMessages', ...)`, and 'newInboxMessages' is only emitted via `model.emit`, not `emitWhenListener`, so this changes does not affect it. Similarly, in my drawer branch, the 'foldersSliceOnChange' is a `model.on()` and 'model.emit()` pairing.

So while this code change has the potential to affect a few pathways, it most likely fixes them vs. breaking them.

Apologies to :asuth for adding to the review queue, but felt he is the most appropriate reviewer given the baser nature of this change and his history with it.
Attachment #8375940 - Flags: review?(bugmail)
Assignee: nobody → jrburke
Target Milestone: --- → 1.4 S1 (14feb)
Comment on attachment 8375940 [details] [review]
GitHub pull request

I concur with your analysis.  emitWhenListener would interact badly with latestOnce and latest(Once)'s semantics accomplish what is desired anyways.  Async/deferred stuff is tricky, yo!
Attachment #8375940 - Flags: review?(bugmail) → review+
Merged gaia master:
https://github.com/mozilla-b2g/gaia/commit/cf24f1963b0ce4950b643a127e6148091fa0887e

from pull request:
https://github.com/mozilla-b2g/gaia/pull/16270
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [p=1]
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: