953277 - Tap 'inbox' with Google Mail account causes all of the emails to be listed twice

Status: VERIFIED FIXED

Description

[Zac C (:zac)]

STR:
1. Configure gmail account into Email app
2. Tap the ≡ icon
3. Tap 'inbox' (Blue background beneath your email address)
4. Wait for mails to download, scroll up to see recent mails duplicated.

Device: Peak
Build ID: 20131221064155
Gaia commit: c1ed307f
Gecko: 29.0a1

Comment 1

[James Burke [:jrburke]]

Created attachment 8379959 [details] [review]
GitHub pull request

This is bug resulted from the code changes in 1.4 to add next/previous navigation, bug 918303, which added `message_cursor`. However the bug is triggered by an existing weakness in `model` that dates back to 1.2 days. To be clear though, this bug does not manifest in 1.2 or 1.3.

The `folder_picker` called `model.changeFolder` even though the folder selection has not changed.

`model` then did not check for if the folder was actually different, and that triggered a 'folder' event to be emitted.

`message_list` received that event in `_folderChanged`, which called `showFolder`. `showFolder` noticed the folders were the same and disregarded the notification, so it did not clear the message list.

`message_cursor` also received the `folder` event from `model`, and it did not check to see if the folder was different and asked for a new messages slice. In 1.2/1.3 code, the messages slice was handled by `message_list`'s `showFolder` method, which did detect for actual folder changes, and it is why the bug is not present there.

The new messages slice in `header_cursor` triggered new messages slice events, which `message_list` was listening for, so it added the duplicates to the list.

So the fix, instead of asking all consumers of that event if the folder is actually different is for `model` to not emit a folder event if the folder selection is not actually different.

Comment 2

[Andrew Sutherland [:asuth]]

Comment on attachment 8379959 [details] [review]
GitHub pull request

r=asuth if you also update the docblock to specify that nothing happens if the given folder is already the current folder.  And maybe add a note that anything that would want to depend on the folder being re-emitted should instead just be using "latest" to get the current folder or keying off some other notification.

Comment 3

[James Burke [:jrburke]]

Updated comment based on review. Did not mention latest or getting every folder call, as that would basically be just intercepting model.changeFolder, but it was hard for me to figure out what use case would need that. But the doc indicates when calling it changes state and events to occur.

Merged on master:
https://github.com/mozilla-b2g/gaia/commit/7d605f4a73851ed7ba82c4e205f26d348c84c03d

from pull request:
https://github.com/mozilla-b2g/gaia/pull/16534

Comment 4

[Edward Chen[:edchen]]

[Environment]

Gaia      71f78f7f68fcf5e23cc5965fee0dad45289c438b
Gecko     https://hg.mozilla.org/mozilla-central/rev/5dc091b7e86f
BuildID   20140304160205
Version   30.0a1

[Result]
PASS


I have verified ticket, the bug fixed so I marked it to "VERIFIED"