Closed Bug 1008477 Opened 6 years ago Closed 6 years ago

[email/UI] Virtual list regression; attachment icon only ever gets set and is never cleared. Because of DOM node reuse results in misleading attachment icon status.

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(blocking-b2g:1.4+, b2g-v1.3 unaffected, b2g-v1.3T unaffected, b2g-v1.4 fixed, b2g-v2.0 fixed)

VERIFIED FIXED
2.0 S2 (23may)
blocking-b2g 1.4+
Tracking Status
b2g-v1.3 --- unaffected
b2g-v1.3T --- unaffected
b2g-v1.4 --- fixed
b2g-v2.0 --- fixed

People

(Reporter: dharris, Assigned: asuth)

References

()

Details

(Keywords: regression, Whiteboard: [openc-1.4-exploratory])

Attachments

(3 files)

Attached file Logcat
Description:
The paper clip attachment icon can be placed on Emails without attachments when scrolling quickly through a list of emails

Prerequisite: Have a large number of emails and at least 1 attachment on one of those emails

Repro Steps:
1) Update a Tarako to BuildID: 20140509014003
2) Open Email app> setup Email
3) Scroll rapidly up and down to the top and bottom of the email list

Actual:
Emails without attachments will show a paper clip next to them indicating that there are attachments on them

Expected:Only emails with attachments on them will show the paper clip next to them

1.3t Environmental Variables:
Device: Tarako 1.3t
BuildID: 20140509014003
Gaia: d07f2dff31e71aed9711d988677fbc5b2c6e83e4
Gecko: 59b3ea7e75fb
Version: 28.1
Firmware Version: SP6821a-Gonk-4.0-4-29


Keywords: Scrolling, Swiping, Fast, Quick, Speed, Incorrect, Icon


Notes: This will not happen with a small E-mail list


Repro frequency: 100%
See attached: Logcat, Firewatch, Video - https://www.youtube.com/watch?edit=vd&v=QHyJz8DtU4c
The issue above DOES occur on Buri 1.4


1.4 Environmental Variables:
Device: Buri 1.4 MOZ
BuildID: 20140508000201
Gaia: 4ce973ef0732b0d52cb043210db598aa176b2ce9
Gecko: 16ab7f6b18f8
Version: 30.0
Firmware Version: v1.2-device.cfg
Good find!

We should update the updateMessageDom variants to use toggle(class, booleanStatus) in all cases rather than using add/remove.

This is an interaction of the existing assumption in updateMessageDom that possessing attachments is an immutable state and the change to reuse DOM nodes.  This was not caught during the review process because it was a secondary implication and we're currently without automated coverage on this aspect; as I finish cleaning up the automated tests I'll ensure that invariants like this are automatically checked.

I'm not sure I'd call this a blocker for v1.4 since we can request approval for it (and I will), but it is a regression, so anyone with blocker+ powers who wants to plus it should feel free.
Assignee: nobody → bugmail
Status: NEW → ASSIGNED
Keywords: regression
Summary: [B2G][E-mail]The paper clip attachment icon can be placed on Emails without attachments → [email/UI] Virtual list regression; attachment icon only ever gets set never, cleared. Because of DOM node reuse results in misleading attachment icon status.
Note that I suspect comment 0's mention of v1.3T may be erroneous.  The video indicates a version of email with bug 796474 fixed which has not landed on v1.3T Tarako and absolutely must not be uplifted there.
I reproduced on my buri running trunk-ish gecko and master.  This fixes it.  I did an audit of other classList.add/remove stuff within the file.  While there are other spots of code that I think would benefit from use of toggle(class, boolean), they are not message related and so I haven't touched them.

(It's probably worth noting that while add/remove is potentially more intuitive in some cases versus passing a negated boolean to toggle, the advantage of it being harder to screw things up would seem to outweigh that.  It may be worth adopting a policy of introducing a named temporary in the cases where toggle's argument requires too much thought.  Or MailAPI could just grow alternate aliases to avoid the need for negation in the first place.)
Attachment #8420490 - Flags: review?(jrburke)
Summary: [email/UI] Virtual list regression; attachment icon only ever gets set never, cleared. Because of DOM node reuse results in misleading attachment icon status. → [email/UI] Virtual list regression; attachment icon only ever gets set and is never cleared. Because of DOM node reuse results in misleading attachment icon status.
Attachment #8420490 - Flags: review?(jrburke) → review+
Whiteboard: [openc-1.4-exploratory] → [openc-1.4-exploratory][c= p=0.5]
Target Milestone: --- → 2.0 S2 (23may)
Whiteboard: [openc-1.4-exploratory][c= p=0.5] → [openc-1.4-exploratory]
Blocking 1.4 for regression from bug 796474.
blocking-b2g: --- → 1.4+
[Environment]
Gaia      917174ee3812a43758bf43f7ba5f9416dcdb2ca8
Gecko     https://hg.mozilla.org/releases/mozilla-b2g28_v1_3t/rev/a3a14fb5ad51
BuildID   20140519164002
Version   28.1
ro.build.version.incremental=eng.cltbld.20140519.201203
ro.build.date=Mon May 19 20:12:10 EDT 2014

[Result]
PASS
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.