1022552 - [WAP Push][CP] Pressing home key or opening other apps will close client provision message directly without any advise and without processing it

Status: RESOLVED FIXED

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

Description

[isabelrios]

Seen on today's (06/09) master buri build:
Gecko-1a316f5
Gaia-deed49c

STR
1. Receive a OMA CP message
2. Open it but do not process it completely
3. Press home key

EXPECTED
The user will be able to come to process that message later

ACTUAL
Once the message is open and although it has not been completely processed if user presses home key, the message is discarded without any advise.

This issue was reported in the past for 1.3 and solved, please see bug 934354. Please see also comment #18 where it is described a better solution than the one already done to be implemented in the future if possible.

Comment 1

[Jason Smith [:jsmith]]

Adding QA Wanted to check 1.4.

Comment 2

[Preeti Raghunath(:Preeti)]

Taking in 2.0 for regressions and cert issue

Comment 3

[Albert [:albert]]

Attachment: Patch

Bug fixed, tests pending.

Comment 4

[Albert [:albert]]

Comment on attachment 8447876 [details]
Patch

The patch implements the proposal of Gabriele in bug 934354 #comment 18

Comment 5

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8447876 [details]
Patch

First of all thank you for your work here and sorry for the delay.

The patch is mostly looking good save for a few issues I've highlighted on GitHub; I'm going to sum them up here:

- wpm_clearNotifications() should do what it says: clear the notifications. Remove the code that also conditionally deletes the message and explicitly delete them instead in the right places.

- mdb_clearByTag() should be called mdb_deleteByTimestamp() for consistency with the rest of the code and because the database has no concept of a tag (that's an abstraction used by the notifications) but knows about message timestamps instead.

- Similarly pm_clear() should be called pm_delete().

- Finally the patch is missing unit-tests for the new functionality and I'm not 100% sure that it still passes the existing ones. Ensure that existing unit-tests are still working and modify them if needed then add new unit-tests covering the additional functionality.

When you've adjusted the patch update your PR, and flag me for review again.

Comment 6

[Albert [:albert]]

Comment on attachment 8447876 [details]
Patch

Changes from comment 5

Comment 7

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8447876 [details]
Patch

The patch structure is good enough and I've left some comments only regarding the function names / organizations and a couple of typos. This is r+ with those comments addressed. Feel free to land this on a green Travis once you've updated the PR.

Comment 8

[Gabriele Svelto [:gsvelto] (PTO)]

Also remember to add r=gsvelto at the end of the patch comment when updating the PR.

Comment 9

[Albert [:albert]]

Changes done and travis green

Comment 10

[Albert [:albert]]

Master https://github.com/mozilla-b2g/gaia/commit/0f922b40725998f100790512140f384be2a7d182

Comment 11

[Ryan VanderMeulen [:RyanVM]]

v2.0: https://github.com/mozilla-b2g/gaia/commit/6200e7b1605932d24a5706b496eaf7b6513d5721

Comment 12

[Wei Gao (Spreadtrum)]

(In reply to Albert [:albert] from comment #10)
> Master
> https://github.com/mozilla-b2g/gaia/commit/
> 0f922b40725998f100790512140f384be2a7d182

Dear Albert

Could your patch be landed on v1.4?
I think v1.4 also needs your patch.

Thanks a great.

Comment 13

[Beatriz Rodríguez [:brg]]

Dear Wei Gao, please ask for 1.4+ flag. Albert will take a look to 1.4 branch in the meantime.

Comment 14

[Wei Gao (Spreadtrum)]

(In reply to Beatriz Rodríguez [:brg] from comment #13)
> Dear Wei Gao, please ask for 1.4+ flag. Albert will take a look to 1.4
> branch in the meantime.

Ok, I see.
I have added this flag.
Thanks.

Comment 15

[Preeti Raghunath(:Preeti)]

Not sure why this one was moved to 1.4. Please see comment 2

Comment 16

[Albert [:albert]]

Attachment: Patch 1.4

I had to modify wappush_test.js in order to adapt the patch for 2.0

The rest of the patch for 2.0 remains the same for 1.4

Comment 17

[Gabriele Svelto [:gsvelto] (PTO)]

(In reply to Preeti Raghunath(:Preeti) from comment #15)
> Not sure why this one was moved to 1.4. Please see comment 2

It was a partner request, see bug 934354 comment 27. Since the fix for that bug was a v1.3t-only affair I suggested asking for approval for this one that applies cleanly to 1.4.

Comment 18

[Gabriele Svelto [:gsvelto] (PTO)]

Comment on attachment 8456654 [details]
Patch 1.4

Looks good to me, thanks.

Comment 19

[Ryan VanderMeulen [:RyanVM]]

v1.4: https://github.com/mozilla-b2g/gaia/commit/edec55eb65b4b8be3e44bedab2949f295784dff1

Comment 20

[Loli (:lolimartinezcr)]

Tested and working in:

Flame
2.1
Gecko-329b2e4
Gaia-da11dba

Flame
2.0
Gecko-2b27bec
Gaia-de28796

Hamachi
Gecko-d7d9838
Gaia-1144cdc

Scenarios:

STR
1. Receive a OMA CP message
2. Open it but do not process it completely
3. Press home key and *also* tested with call incoming

EXPECTED
The user will be able to come to process that message later