Closed Bug 953277 Opened 9 years ago Closed 9 years ago

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


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

Gonk (Firefox OS)
Not set


(blocking-b2g:1.4+, b2g-v1.4 fixed)

1.4 S2 (28feb)
blocking-b2g 1.4+
Tracking Status
b2g-v1.4 --- fixed


(Reporter: zcampbell, Assigned: jrburke)


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


(1 file)

46 bytes, text/x-github-pull-request
: review+
Details | Review
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
Component: Gaia::Settings → Gaia::E-Mail
Assignee: nobody → jrburke
Target Milestone: --- → 1.4 S2 (28feb)
Attached file 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.
Attachment #8379959 - Flags: review?(bugmail)
blocking-b2g: --- → 1.4?
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.
Attachment #8379959 - Flags: review?(bugmail) → review+
blocking-b2g: 1.4? → 1.4+
Keywords: regression
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:

from pull request:
Closed: 9 years ago
Resolution: --- → FIXED
Whiteboard: [p=1]

Gaia      71f78f7f68fcf5e23cc5965fee0dad45289c438b
BuildID   20140304160205
Version   30.0a1


I have verified ticket, the bug fixed so I marked it to "VERIFIED"
You need to log in before you can comment on or make changes to this bug.