885264 - [SMS][MMS] Ensure Carrier is not displayed for manually entered (unknown contact) recipients

Status: VERIFIED FIXED

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

Description

[isabelrios]

Attachment: Message to one number bug info displayed is about another one

Unagi device, build:
Gecko-856e202
Gaia-aea6834

PROCEDURE
There should be at least one contact stored in the address book with phone number and type
1. Send a message to that contact
2. Create a new message
3. Send the new message to a number not stored in the addressbook
4. Go to messages list view and open the message just sent

EXPECTED
Since the number is not a known one, there should not be any info just below the header of the message thread view

ACTUAL
Under the header of the message sent to a non known number it is displayed the info of the contact to whom the other message was previously sent.

Plase see screenshot attached

Comment 1

[Rick Waldron [:rwaldron]]

Attachment: Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10543

Comment 2

[Julien Wajsberg [:julienw]]

Ayman, I don't get where that "Carrier unknown" banner comes from. My understanding was that we should not display any information about the carrier when we have no information, and no banner at all if the number belongs to no contact (this is in the SMS wireframes). Could you please shed some light ? :)

Thanks !

Comment 3

[ayman maat :maat]

(In reply to Julien Wajsberg [:julienw] from comment #2)
> Ayman, I don't get where that "Carrier unknown" banner comes from. My
> understanding was that we should not display any information about the
> carrier when we have no information, and no banner at all if the number
> belongs to no contact (this is in the SMS wireframes). Could you please shed
> some light ? :)
> 
> Thanks !

Hi Julien

Thanks for pinging me. Yes you are correct in your analysis and correct to reference the SMS specs as this is 'core' behavior and therefore not duplicated in the MMS specification. So Referencing:

wireframe pack: HTML5_SMS_20121212_R2S1_V8.0
page: 6
annotation: 10

	- if the contact is not in the users address book, do not display type of phone and carrier.

...I have absolutely no idea where the "carrier unknown" banner comes from. Its never been specified and is completely incorrect behavior as it is delivering the user irrelevant information and just creating noise on the screen. Therefore behavior needs to be aligned to specification

Irrespective of the fact that this is incorrect behavior one thing i am noting when i look at attachment : https://bug885264.bugzilla.mozilla.org/attachment.cgi?id=765257 is that the phone number in the header '620989920' does not correspond to the phone number in the carrier banner '646817295'... why is this?

Comment 4

[isabelrios]

> 
> Irrespective of the fact that this is incorrect behavior one thing i am
> noting when i look at attachment :
> https://bug885264.bugzilla.mozilla.org/attachment.cgi?id=765257 is that the
> phone number in the header '620989920' does not correspond to the phone
> number in the carrier banner '646817295'... why is this?

Hi, that is the point of this bug. The information shown there is not corresponding to the number the message is written for.

The summary of this bug was changed maybe because it was not clear enough, but it is not about 'unknown carrier' banner. The bug is about information that is kept in carrier and number field, from one message to another when you firstly send a message to a contact and then to a number entered manually.

Please ping me if the stpes/actual is still not clear.

Comment 5

[ayman maat :maat]

Hi Isabel

..Oh ok I completely missed that! ..then in that case we have two bugs:

bug 1) 'Ensure "Carrier Unknown" is *NOT* displayed for manually entered recipients as it contradicts specification, what has already been developed within the messaging app and degrades the UX'

 - is there a bug open for this or should we adjust this title of bug? 

bug 2) Referencing Comment 0 and comment 4

 - it is not clear to me how to reproduce this. Could you confirm the steps for me please, or does this new "carrier unknown" banner bug we have now mask reproduction of the initial bug?

Comment 6

[isabelrios]

(In reply to ayman maat :maat from comment #5)
> Hi Isabel
> 
> ..Oh ok I completely missed that! ..then in that case we have two bugs:
> 
> bug 1) 'Ensure "Carrier Unknown" is *NOT* displayed for manually entered
> recipients as it contradicts specification, what has already been developed
> within the messaging app and degrades the UX'
> 
>  - is there a bug open for this or should we adjust this title of bug? 

For phone numbers entered manually nothing is displayed below the header with the number. So I guess that is right and there is not a bug about that (at least so far with the build checked)

> 
> bug 2) Referencing Comment 0 and comment 4
> 
>  - it is not clear to me how to reproduce this. Could you confirm the steps
> for me please, or does this new "carrier unknown" banner bug we have now
> mask reproduction of the initial bug?

When I filed the bug, it was very easy to reproduce the bug following the steps given in the description.
With today's build (Gecko-2ff3151.Gaia-d8d189b) I cannot reproduce the bug. I will look at this more deeply and if cannot reproduce it, will close as WFM.

Thanks

Comment 7

[Julien Wajsberg [:julienw]]

(In reply to ayman maat :maat from comment #5)
> Hi Isabel
> 
> ..Oh ok I completely missed that! ..then in that case we have two bugs:
> 
> bug 1) 'Ensure "Carrier Unknown" is *NOT* displayed for manually entered
> recipients as it contradicts specification, what has already been developed
> within the messaging app and degrades the UX'
> 
>  - is there a bug open for this or should we adjust this title of bug? 


I'm quite sure Rick can do it in this bug. Rick, would you agree with this ? 

> bug 2) Referencing Comment 0 and comment 4
> 
>  - it is not clear to me how to reproduce this. Could you confirm the steps
> for me please, or does this new "carrier unknown" banner bug we have now
> mask reproduction of the initial bug?

it happens when you first send a sms to a known contact (and then the number is displayed in the "banner line"), and then you send another sms to an unknown contact (the number is then displayed in the header), but we don't empty correctly the banner line. I think at first we were hiding it as expected and that's why we were not changing the text inside. I think we can do that again to fix this bug too.

Comment 8

[Julien Wajsberg [:julienw]]

Isabel, could you please check if it's different on 1.1 and master ?

Comment 9

[isabelrios]

(In reply to Julien Wajsberg [:julienw] from comment #8)
> Isabel, could you please check if it's different on 1.1 and master ?

Yes, I just checked it and in master you can see the problem as you described above.

The behaviour is different then in v1-train and master. Currently:
In v1-train not reproducible
In master bug reproducible

Comment 10

[ayman maat :maat]

ok cool. thanks for your input Julien and Isabel. Just for my own clarity i am going post the PEA for these two interrelated issues as i reproduce them:

**PATH**
1) ensure that there is at least one contact with a phone number associated to it in the contact list.
2) open message app
3) select new message
4) in the 'to field' enter a phone number that is not in the contact list
5) enter some text in the message field
6) select send

**EXPECTED**
6.1e) message tread is created with phone number in header and no 'type / carrier' banner

**ACTUAL**
6.1a) message tread is created with phone number in header and a 'type / carrier' banner containing the text "unknown carrier"

**PATH**
7) return to message thread listing 
8) select new message
9) open the contacts list from the 'to field'
10) select a contact from contacts list
11) write something in 'to field'
12) select send
13) return to message thread listing 
14) open the message thread that was created in step 6)

**EXPECTED**
14.1e) message tread is presented with phone number in header and no 'type / carrier' banner

**ACTUAL**
14.1a) message tread is presented with phone number in header and a 'type / carrier' banner now containing the type and carrier of the contact selected in step 10)

Comment 11

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #7)
> (In reply to ayman maat :maat from comment #5)
> > Hi Isabel
> > 
> > ..Oh ok I completely missed that! ..then in that case we have two bugs:
> > 
> > bug 1) 'Ensure "Carrier Unknown" is *NOT* displayed for manually entered
> > recipients as it contradicts specification, what has already been developed
> > within the messaging app and degrades the UX'
> > 
> >  - is there a bug open for this or should we adjust this title of bug? 
> 
> 
> I'm quite sure Rick can do it in this bug. Rick, would you agree with this ? 

That's what my version of the patch did... I'm just changing it back.

> 
> > bug 2) Referencing Comment 0 and comment 4
> > 
> >  - it is not clear to me how to reproduce this. Could you confirm the steps
> > for me please, or does this new "carrier unknown" banner bug we have now
> > mask reproduction of the initial bug?
> 
> it happens when you first send a sms to a known contact (and then the number
> is displayed in the "banner line"), and then you send another sms to an
> unknown contact (the number is then displayed in the header), but we don't
> empty correctly the banner line. I think at first we were hiding it as
> expected and that's why we were not changing the text inside. I think we can
> do that again to fix this bug too.

Comment 12

[Rick Waldron [:rwaldron]]

(In reply to ayman maat :maat from comment #10)
> ok cool. thanks for your input Julien and Isabel. Just for my own clarity i
> am going post the PEA for these two interrelated issues as i reproduce them:
> 
> **PATH**
> 1) ensure that there is at least one contact with a phone number associated
> to it in the contact list.
> 2) open message app
> 3) select new message
> 4) in the 'to field' enter a phone number that is not in the contact list
> 5) enter some text in the message field
> 6) select send
> 
> **EXPECTED**
> 6.1e) message tread is created with phone number in header and no 'type /
> carrier' banner
> 
> **ACTUAL**
> 6.1a) message tread is created with phone number in header and a 'type /
> carrier' banner containing the text "unknown carrier"
> 
> **PATH**
> 7) return to message thread listing 
> 8) select new message
> 9) open the contacts list from the 'to field'
> 10) select a contact from contacts list
> 11) write something in 'to field'
> 12) select send
> 13) return to message thread listing 
> 14) open the message thread that was created in step 6)
> 
> **EXPECTED**
> 14.1e) message tread is presented with phone number in header and no 'type /
> carrier' banner
> 
> **ACTUAL**
> 14.1a) message tread is presented with phone number in header and a 'type /
> carrier' banner now containing the type and carrier of the contact selected
> in step 10)


Is this the new format? "Type / Carrier"? Or is it "Type, Carrier"?

Comment 13

[Julien Wajsberg [:julienw]]

(In reply to Rick Waldron from comment #12)

> Is this the new format? "Type / Carrier"? Or is it "Type, Carrier"?

"Type / Carrier" is the UX way of saying both information needs to be displayed ;) that's still not visual !

Comment 14

[Rick Waldron [:rwaldron]]

(In reply to Julien Wajsberg [:julienw] from comment #13)
> (In reply to Rick Waldron from comment #12)
> 
> > Is this the new format? "Type / Carrier"? Or is it "Type, Carrier"?
> 
> "Type / Carrier" is the UX way of saying both information needs to be
> displayed ;) that's still not visual !


The quotes are what lead me to 
(In reply to Julien Wajsberg [:julienw] from comment #13)
> (In reply to Rick Waldron from comment #12)
> 
> > Is this the new format? "Type / Carrier"? Or is it "Type, Carrier"?
> 
> "Type / Carrier" is the UX way of saying both information needs to be
> displayed ;) that's still not visual !

Thanks for the clarification

Comment 15

[Rick Waldron [:rwaldron]]

changing to better match other ticket subjects

Comment 17

[Julien Wajsberg [:julienw]]

Comment on attachment 766055 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10543

reviewed on github

basically nothing much to say about the code itself, but there is a case that's in the spec and that we don't do know, I feel like we should do it right in this bug so that we can close that topic for good.

Comment 18

[Julien Wajsberg [:julienw]]

Comment on attachment 766055 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10543

r=me with the nit and test changes

of course, please check that the test passes before merging :)

Comment 19

[Rick Waldron [:rwaldron]]

Landed https://github.com/mozilla-b2g/gaia/commit/eb02e6102d143f277f19b99323bd1f8d23963248

Comment 20

[Gareth Aye [:gaye] (back from PTO)]

Uplifted eb02e6102d143f277f19b99323bd1f8d23963248 to:
v1-train: 22e305df921f542cc7bdacac8b74c164855a4b8c

Comment 21

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

v1.1.0hd: 22e305df921f542cc7bdacac8b74c164855a4b8c

Comment 22

[isabelrios]

Unknown carrier is not shown.
It is not possible to reproduce either the problem on comment0

Verified with unagi device 06/28 build:
Gecko-db60841
Gaia-508cfc8