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

RESOLVED WONTFIX

Status

RESOLVED WONTFIX
6 years ago
2 years ago

People

(Reporter: b.paloma, Unassigned)

Tracking

unspecified
Dependency tree / graph

Firefox Tracking Flags

(b2g18?)

Details

(Whiteboard: [sms-most-wanted])

(Reporter)

Description

6 years ago
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.
(Reporter)

Updated

6 years ago
Keywords: perf

Updated

6 years ago
Status: UNCONFIRMED → NEW
Ever confirmed: true
(Reporter)

Updated

6 years ago
tracking-b2g18: --- → ?
Summary: [SMS] Select All button doesn't work correctly when item count>2000 → [SMS] Select All button doesn't work correctly when item count>500
Assignee: nobody → waldron.rick
Duplicate of this bug: 894688
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+
Summary: [SMS] Select All button doesn't work correctly when item count>500 → [SMS][MMS] "Select All" + "Delete" should delete all messages in a thread, rendered or not
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?
(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...
(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.
(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.
(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

Updated

6 years ago
Whiteboard: [c= p= s= u=]

Updated

5 years ago
Keywords: perf
Whiteboard: [c= p= s= u=]
See Also: → bug 1019455
Depends on: 1074732
Status: NEW → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1084298
Status: RESOLVED → REOPENED
Depends on: 1084298
Resolution: DUPLICATE → ---
Assignee: waldron.rick → nobody
Whiteboard: [sms-most-wanted]
Mass closing of Gaia::SMS bugs. End of an era :(
Status: REOPENED → RESOLVED
Last Resolved: 4 years ago2 years ago
Resolution: --- → WONTFIX
Mass closing of Gaia::SMS bugs. End of an era :(
You need to log in before you can comment on or make changes to this bug.