880647 - [SMS][MMS] "Select All" + "Delete" should delete all messages in a thread, rendered or not

Status: RESOLVED WONTFIX

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

Description

[b.paloma]

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:21.0) Gecko/20100101 Firefox/21.0 (Beta/Release)
Build ID: 20130511120803

Steps to reproduce:


GECKO: 65bbcee
GAIA: 13c6246

DESCRIPTION:

If we press "Select all", all sms aren't selected to delete and the "Select all"  button continues activated, and if you press it again and again, the number of selected sms is increased. Apparently this happens because all sms aren't loaded yet.

> I attach a video of the problem on this link (it's too big to attach here): 
https://www.dropbox.com/s/euya88q1o6grodd/video8.mp4

STEPS:
1) Launch SMS App
2) Go to Edit mode
3) Tap on Select All



Actual results:

All SMS aren't checked because all SMS aren't loaded.


Expected results:

All SMS are checked.

Comment 2

[Rick Waldron [:rwaldron]]

I originally reported the following in bug 894688, because I didn't find this issue when searching for dups (thanks to jugglinmike for tracking it down).

The bug title has been edited because the problem appeared when using the medium reference workload:

- BIG-THREAD-MIXED, 174 messages
- BIG-THREAD-MMS, 194 messages
- BIG-THREAD-SMS, 146 messages


I then confirmed with the light reference workload:

- BIG-THREAD-MIXED, 70 messages
- BIG-THREAD-MMS, 54 messages
- BIG-THREAD-SMS, 64 messages


The STR can be observed with either workload.

STR: 

1. Create (at least) a medium workload `make reference-workload-medium APP=sms;`
2. Open Messages app, tap the first conversation "BIG-THREAD-MIXED"
3. As soon as the messages view opens, tap "Edit" > "Select All". 
(The app may display something like "13 Selected", click "Select All" again and it may say "25 Selected" and so on)

In summary, the only way to "Select All" + "Delete" is to wait for all messages to be rendered, because only those messages currently rendered in the DOM can be "Selected" for deletion. The reason for this behaviour is a pathological coupling with the DOM.


Here's a rough outline of intended solution:

1. Messages should be represented in program memory as data objects, ie. a Message type.
2. Threads should be represented in program memory as data objects. ie. a Thread type.
3. Instances of Message type should weakly correspond to a representative DOM node
3. Collections of Message instances correspond to a Thread instance (thread.id === message.threadId)
4. Thread instances should track **Delete Intention** via "deleteAll" property (or similar)
5. If the current Thread instance's "deleteAll" property becomes true:
    - Stop the render messages cursor (prevents further rendering of messages for this thread)
    - Use the Thread delete mechanism (ie. ThreadListUI.delete())


**Delete Intention**

1. Let deleteAll be *false*.
2. If User taps "X"
    - Return deleteAll.
3. If User selects individual messages
    - If User taps "X"
        * See Step 2.
    - If User taps "Delete"
        * Return deleteAll.
    - If User taps "Select All"
        * See Step 4.
4. If User taps "Select All" 
    - If User taps "X"
        * See Step 2.
    - If User taps "Delete"
        * Let deleteAll be *true*.
        * Return deleteAll.
    - If User taps "Deselect All"
        * See Step 3.




Requesting Leo+

Comment 3

[Borja Salguero [:borjasalguero]]

As we are rendering using a cursor, I'll move forward using the easiest solution. Why dont we disable the button 'edit' until rendering the entire list?

Comment 4

[[:fabrice] Fabrice Desré]

(In reply to Borja Salguero [:borjasalguero] from comment #3)
> As we are rendering using a cursor, I'll move forward using the easiest
> solution. Why dont we disable the button 'edit' until rendering the entire
> list?

That looks like pretty poor UX to me...

Comment 5

[Borja Salguero [:borjasalguero]]

(In reply to Fabrice Desré [:fabrice] from comment #4)
> (In reply to Borja Salguero [:borjasalguero] from comment #3)
> > As we are rendering using a cursor, I'll move forward using the easiest
> > solution. Why dont we disable the button 'edit' until rendering the entire
> > list?
> 
> That looks like pretty poor UX to me...

This is the same solution agreed with UX in the call log... That's why the proposal.

Comment 6

[Steve Chung [:steveck]]

(In reply to Borja Salguero [:borjasalguero] from comment #3)
> As we are rendering using a cursor, I'll move forward using the easiest
> solution. Why dont we disable the button 'edit' until rendering the entire
> list?

cc gecko devs for more suggestion in performance tuning.

This method is the easiest way and safe, but user experience could be an issue because user have to wait until load complete(especially it may take lots of time) without any clear information why the button is disabled. Stop the cursor and delete the entire thread seems a reasonable solution here.

Some simple ideas bases on the discussion with gecko devs: Instead of having another deleteThread api, we could change a original deleteMessage api to accept smsfilter just like getMessages api, therefore we can delete id/thread flexibly.

Comment 7

[Rick Waldron [:rwaldron]]

(In reply to Steve Chung from comment #6)
> (In reply to Borja Salguero [:borjasalguero] from comment #3)
> > As we are rendering using a cursor, I'll move forward using the easiest
> > solution. Why dont we disable the button 'edit' until rendering the entire
> > list?
> 
> cc gecko devs for more suggestion in performance tuning.
> 
> This method is the easiest way and safe, but user experience could be an
> issue because user have to wait until load complete(especially it may take
> lots of time) without any clear information why the button is disabled. Stop
> the cursor and delete the entire thread seems a reasonable solution here.
> 
> Some simple ideas bases on the discussion with gecko devs: Instead of having
> another deleteThread api, we could change a original deleteMessage api to
> accept smsfilter just like getMessages api, therefore we can delete
> id/thread flexibly.

I agree that deleteMessages should accept a "MozSmsFilter", eg. { threadId: ... } would make deleting all messages in a thread trivial (vs. collecting the the ids and passing an array)

For now, if you test my WIP branch, you'll see that it solves the problem without blocking the user from the Edit button https://github.com/rwldrn/gaia/commit/8e2c4ef43ecccb14d2097adc36f1df164ec31cae

Comment 9

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(

Comment 10

[Julien Wajsberg [:julienw]]

Mass closing of Gaia::SMS bugs. End of an era :(