934916 - [WAP push][CP] If a CP message is opened, newly coming CP or WAP push messages can still be opened but content is not displayed.

Status: RESOLVED FIXED

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

Description

[Enpei]

Attachment: time stamp: 15:05, 11/5

When a CP message is opened, system still allow users to open newly arrived WAP push messages which turns out new message content will not be shown and still they will be cleared from utility tray. 

* Build Number                
Gaia:     1aee772a384f1ed1148f08c6c7df45d2fe35506e
Gecko:    http://hg.mozilla.org/mozilla-central/rev/b4143e04bea1
BuildID   20131104044747
Version   28.0a1

* Reproduce Steps
1. Send a CP message to DUT, open it and stay in PIN requesting page.
2. Send other CP or WAP push message to DUT.
3. Scroll down utility try and open new message.

* Expected Result
1. If FxOS allows user to read new message, content should be displayed.
  - If this will close original CP message, quiting warning message should pop up to let user choose whether they want to exit original CP message or not.
    - If user select no, then back to original CP message, and new message notification will still keep in utility tray.

2. If FxOS does not allow user to read new message, it should have UI to block this action and new message notification will still keep in utility tray.

3. After above actions, if user press "x" button of original CP message, it should still pop up warning message as current design acts now. (If this should have another bug for tracking, please let me know)

* Actual Result
1. Looks like FxOS allow users to open new message, yet new content will not be displayed, still keeps in original PIN requesting page.
2. When users press "x" button of original CP, it will exit directly without popping up warning message like current design.

* Occurrence rate
100%

Comment 1

[Enpei]

Update expected result

* Expected Result
1. If FxOS allows user to read new message, content should be displayed.
  - If this will close original CP message, quiting warning message should pop up to let user choose whether they want to exit original CP message or not.
    - If user select no, then back to original CP message, and new message notification will still keep in utility tray.
    - If user select yes, then new message content will be displayed, and original CP message will be closed.
  - If this will not close original CP message(which might not be possible since we have bug 934354), new message will be opened with its content shown. After exit this message, user can see original CP message or user needs to reopen it from utility tray.

2. If FxOS does not allow user to read new message, it should have UI to block this action and new message notification will still keep in utility tray, which allows user to open it after exit original CP message later.

3. After above actions, if user press "x" button of original CP message, it should still pop up warning message as current design acts now. (If this should have another bug for tracking, please let me know)

Comment 2

[Enpei]

Change block flag to koi? since WAP CP (bug 917312, koi+) is for v1.2.

Comment 3

[Kai-Chih Hu [:khu]]

Joe, can Comms team work on it and land it to v1.3? Thanks.

Comment 4

[bhavana bajaj [:bajaj]]

It sounds like a busted new feature, hence blocking on this.

Comment 5

[David Palomino [:dpalomino]]

Hi, 

I think we should block on this for koi, as AFAIK, it's a common scenario to receive multiple SMS for one single configuration action driven by Customer Care department (for instance, two different messages for changing configuration for Internet APN and/or MMS, or other similar). 

Thnks, 
David

Comment 6

[David Palomino [:dpalomino]]

Hi, 

After some internal discussions, in the same way as discussed in bug #934354, we think it could be a bit late to include this change for koi now. As it is an important issue, we suggest to keep the focus on it, and try at least to have this for 1.3. 

So, renominating from koi+ to koi?, and suggesting to nominate then to 1.3?

Sorry for the misunderstanding... 

Thanks!
David

Comment 7

[Jason Smith [:jsmith]]

See https://bugzilla.mozilla.org/show_bug.cgi?id=917312#c88.

Comment 8

[Jason Smith [:jsmith]]

Not a system bug - moving into the Gaia component, until we get a bugzilla component created for WAP Push.

Comment 9

[Joe Cheng [:jcheng] (please needinfo)]

triage: 1.3+ to work on in convergence period

Comment 10

[Jean Gong :jgong]

PM triaged this bug and believes that it should be a blocker.

Comment 11

[Kai-Chih Hu [:khu]]

My understanding is, this may need Comms team's help and/or System app's help. Joe/Ivan, is this something under your radar? Thank you.

Comment 12

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Kevin Hu [:khu] from comment #11)
> My understanding is, this may need Comms team's help and/or System app's
> help. Joe/Ivan, is this something under your radar? Thank you.

My plan is to take it as soon as possible as I have some other 1.3+ bugs. Once I have time I'll take care of this bug. If in the mean time anyone else wants to take that would be great.

Comment 13

[Ivan Tsay (:ITsay)]

Based on comment 12, this will be under Comms team radar.

Comment 14

[Gabriele Svelto [:gsvelto]]

Moved top the Gaia::Wappush component.

Comment 15

[Jean Gong :jgong]

PM team looked this bug and believe that it should be a blocker.

Comment 16

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Enpei from comment #1)
> Update expected result
> 
> * Expected Result
> 1. If FxOS allows user to read new message, content should be displayed.
>   - If this will close original CP message, quiting warning message should
> pop up to let user choose whether they want to exit original CP message or
> not.
>     - If user select no, then back to original CP message, and new message
> notification will still keep in utility tray.
>     - If user select yes, then new message content will be displayed, and
> original CP message will be closed.
>   - If this will not close original CP message(which might not be possible
> since we have bug 934354), new message will be opened with its content
> shown. After exit this message, user can see original CP message or user
> needs to reopen it from utility tray.
> 
> 2. If FxOS does not allow user to read new message, it should have UI to
> block this action and new message notification will still keep in utility
> tray, which allows user to open it after exit original CP message later.
> 
> 3. After above actions, if user press "x" button of original CP message, it
> should still pop up warning message as current design acts now. (If this
> should have another bug for tracking, please let me know)

The needs above sounds like a new feature to me. I agree that current UX while reading/processing messages is quite poor and leads to errors but IHMO we should pause a bit a think about what we would need to do here is more than fixing a bug. I mean the UX requested in comment #1 needs some logic underneath to handle all the messages, re-store them, re-send notifications, etc. etc. This remembers me the new notification API (won't be uplifted) is already present in master so I guess we probably need to work on this as a 1.3-only work/patch and see if the older notification API is valid or not (has the support we need). Having said that, Would it be possible to move this to 1.4 release?

Comment 17

[Beatriz Rodríguez [:brg]]

This is a certification blocker in 1.3.
There should be a mecanism to avoid the user to open an empty message in the utility tray. I do not know which of the options is easier or safer but we need a change of this behaviour.

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #16)
> Having said that, Would it be possible to move
> this to 1.4 release?
Unfortunately, no. Sorry.

Comment 18

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Beatriz Rodríguez [:brg] from comment #17)
> This is a certification blocker in 1.3.
> There should be a mecanism to avoid the user to open an empty message in the
> utility tray.

I have not seen empty messages in the test I'm performing so It would be a matter of thinking about what to do with open messages if the user wants to open another new one from the notification area.

Enpei, in comment #0 you said "When a CP message is opened, system still allow users to open newly arrived WAP push messages which turns out new message content will not be shown and still they will be cleared from utility tray." How did you manage the message not been shown?, could you provide the STR, a log or even a video with the issue please? Thanks!

Comment 19

[José Antonio Olivera Ortega [:jaoo]]

At comment #1 Enpei provided some options about how to handle new messages while processing previous arrived ones. We would need the input/feedback from UX team to really know what is the right thing to do here. Requesting information at UX team then.

Comment 20

[José Antonio Olivera Ortega [:jaoo]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #18)

> Enpei, in comment #0 you said "When a CP message is opened, system still
> allow users to open newly arrived WAP push messages which turns out new
> message content will not be shown and still they will be cleared from
> utility tray."

I'm able to reproduce the issue. Now I understand what you meant above. Sorry for the noise.

Comment 21

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v1

Gabriele, this patch addresses one of the issues commented in the bug. If a CP message is opened, newly coming CP or WAP push messages can still be opened but content is not displayed. What actually happens is that the CP screen is not updated correctly (I'll update the title of this bug). On the other hand the another issue commented in this bug is that we should provide an better UX for handling the messages. I mean what to do when the user open a new message while processing another one and so on. I see these issues as two different bugs and I wonder if we could slip them up, what do you think? That way we could take care of the first bug here and open a new bug for the UX thing.

The patch here allows the UI to be updated correctly in case the user opens a new message while processing another one. We just needed to show/hide the pin field and help message in case there is no need to request the user a PIN code for authenticating the message. Could you take a look a it and provide some feedback please? Thanks.

Comment 22

[Enpei]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #20)
> (In reply to José Antonio Olivera Ortega [:jaoo] from comment #18)
> 
> > Enpei, in comment #0 you said "When a CP message is opened, system still
> > allow users to open newly arrived WAP push messages which turns out new
> > message content will not be shown and still they will be cleared from
> > utility tray."
> 
> I'm able to reproduce the issue. Now I understand what you meant above.
> Sorry for the noise.

Thanks and sorry that I may not describe the issue well enough. Please let me know if it's needed to separate bugs based on comment 21.

Comment 23

[Dietrich Ayala (:dietrich)]

Assigning to jaoo since he put a patch up ;)

Comment 24

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8369588 [details] [diff] [review]
v1

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

Looks good; in general we should make sure that initialization paths for the various panels always work, independently of the state of the application.

Comment 25

[Wesley Huang [:wesley_huang] (EPM) (NI me)]

Hi José,
It's me asking for ETA (for 1.3) again.
Please just mark ETA date on the whiteboard. example-> ETA:2/7

Comment 26

[Kai-Chih Hu [:khu]]

Can we get ETA to fix this bug? Thank you.

Comment 27

[Noemí Freire (:noemi)]

Hi,

Per comment #21, there are 2 different issues to be handled within this bug, let's focus on solving the issue related to properly shown new incoming CP/WAP push messages when there is one already opened within this bug and create a follow up bug (Bug 968160) to provide a better UX for handling several messages.

Comment 28

[José Antonio Olivera Ortega [:jaoo]]

Comment on attachment 8369588 [details] [diff] [review]
v1

Gabriel, as Noemí commented at comment #21 as I discussed in other comments let's focus here on showing the content of the message correctly when opening a new message through its notification while processing another one previously received. Would you review the patch now please? Thanks.

Comment 29

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8369588 [details] [diff] [review]
v1

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

r+ with an additional test in the CP testsuite that verifies that the PIN field is properly visualized / hidden:

https://github.com/mozilla-b2g/gaia/blob/d128068889bb44b45a976e16b89e0bcaa688f24a/apps/wappush/test/unit/wappush_test.js#L207

Once you've added the test merge at will as soon as Travis is green.

Comment 30

[José Antonio Olivera Ortega [:jaoo]]

Attachment: Pointer to Github PR https://github.com/mozilla-b2g/gaia/pull/15993

Comment 31

[José Antonio Olivera Ortega [:jaoo]]

Attachment: v2. r=gsvelto

(In reply to Gabriele Svelto [:gsvelto] from comment #29)
> Comment on attachment 8369588 [details] [diff] [review]
> v1
> 
> Review of attachment 8369588 [details] [diff] [review]:
> -----------------------------------------------------------------

Thanks for the review Gabriele.
 
> r+ with an additional test in the CP testsuite that verifies that the PIN
> field is properly visualized / hidden:

Test added.

Carrying out r=gsvelto

Comment 32

[José Antonio Olivera Ortega [:jaoo]]

Travis went green and everything seems to work fine. I've tested the patch with a real server and ran the unit test locally without any failure.

Patch landed on Gaia master branch at https://github.com/mozilla-b2g/gaia/commit/3ad11b1058a0ec4e9a64d7a786e7863b7bff27e5

Comment 33

[Gabriele Svelto [:gsvelto]]

(In reply to José Antonio Olivera Ortega [:jaoo] from comment #32)
> Travis went green and everything seems to work fine. I've tested the patch
> with a real server and ran the unit test locally without any failure.

Excellent, thanks!

Comment 34

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

I was not able to uplift this bug to v1.3.  If this bug has dependencies which are not marked in this bug, please comment on this bug.  If this bug depends on patches that aren't approved for v1.3, we need to re-evaluate the approval.  Otherwise, if this is just a merge conflict, you might be able to resolve it with:

  git checkout v1.3
  git cherry-pick -x -m1 3ad11b1058a0ec4e9a64d7a786e7863b7bff27e5
  <RESOLVE MERGE CONFLICTS>
  git commit

Comment 35

[José Antonio Olivera Ortega [:jaoo]]

Landed on Gaia v1.3 branch at https://github.com/mozilla-b2g/gaia/commit/c26137c59c4d39beadc815c3cd92f0483fac6266