876803 - Thread List UI: Select some threads and press delete

Status: RESOLVED FIXED

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

Description

[Dietrich Ayala (:dietrich)]

STR

    1- Access to SMS app

    2- Press edit button

    3- Select multiple conversations

    Expected Result: The Selected conversations are deleted successfull

    gets stuck at deleteing.... screen

    does that work for individual messages ? messages yes, threads no

    E/GeckoConsole(  862): [JavaScript Error: "threadRecord is undefined" {file: "jar:file:///system/b2g/omni.ja!/components/MobileMessageDatabaseService.js" line: 2193}] looks like a gecko problem ? or maybe we give wrong informations... but imho gecko should never fail like that either - agree

Comment 1

[Dietrich Ayala (:dietrich)]

Regression from mms code, will fix post-merge.

Comment 2

[Dietrich Ayala (:dietrich)]

(In reply to Dietrich Ayala (:dietrich) from comment #1)
> Regression from mms code, will fix post-merge.

I commented on the wrong bug. This one cannot be fixed post-merge, must be fixed before merge occurs.

Corey's got a patch in progress now.

Comment 3

[Corey Frang [:gnarf]]

Attachment: patch v1

The old logic wasn't properly keeping track of variables for delete thread.

Also, we shouldn't hide the WaitingScreen or require a re-render from MessageManager if we fix ThreadListUI.removeThread

Comment 4

[Corey Frang [:gnarf]]

Attachment: patch v2

fixed a bug in header removal

Comment 5

[Corey Frang [:gnarf]]

I'm going to ammend one more line into the FixedHeader.refresh() - I assumed this method would also update headers, but it doesn't - https://gist.github.com/gnarf37/7082a4befe0f7f68ebd4

Comment 6

[Steve Chung [:steveck]]

Comment on attachment 755081 [details] [diff] [review]
patch v2

Review of attachment 755081 [details] [diff] [review]:
-----------------------------------------------------------------

::: apps/sms/js/message_manager.js
@@ +240,5 @@
>          //Keep the  visible button the :last-child
>          var editButton = document.getElementById('icon-edit');
>          editButton.parentNode.appendChild(editButton);
>          if (mainWrapper.classList.contains('edit')) {
>            mainWrapper.classList.remove('edit');

I think it's find to remove the re-render here, but we have to update the thread view status(edit button and no message background) after edit.

::: apps/sms/js/thread_list_ui.js
@@ +181,5 @@
>      var li = document.getElementById('thread-' + threadId);
> +    var parent = li.parentNode;
> +    parent.removeChild(li);
> +
> +    // remove the header and the ul for a now empty list

"new" empty list?

Comment 7

[Corey Frang [:gnarf]]

Attachment: patch v3

Comment 8

[Steve Chung [:steveck]]

Comment on attachment 755164 [details] [diff] [review]
patch v3

Thanks for the update. r=me with the small typo fixing.

Comment 9

[Corey Frang [:gnarf]]

master: https://github.com/mozilla-b2g/gaia/commit/451796de73fb57368fa419e25efdc86e7ecebb2f

Comment 10

[John Ford [:jhford] CET/CEST Berlin Time]

v1-train: 4694e3b

Comment 11

[Julien Wajsberg [:julienw]]

Corey, this broke the linter.

Please fix it today (on both branches).

Comment 12

[Julien Wajsberg [:julienw]]

Attachment: fix linter patch

linter fix

see also PR https://github.com/mozilla-b2g/gaia/pull/10071

Comment 13

[Julien Wajsberg [:julienw]]

master: cc4b103ab2ffe3dc65602bd12dcfb4952f1d4602
v1-train: 8f5ab7bfd4a2921aab4e2de11e0d79a29c1bb062