899003 - [Buri][Buri][SMS] Long press on number in message: menu includes "Call" option

Status: RESOLVED FIXED

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

Description

[sync-1]

AU_LINUX_GECKO_ICS_STRAWBERRY.01.01.00.019.171
 Firefox os  v1.1
 Mozilla build ID:20130722070207
 
 Created an attachment (id=472390)
 0727
 
 DEFECT DESCRIPTION:
 Less a option "Call" when long press the number
 
 REPRODUCING PROCEDURES:
 Precondition:
 The number you send is not in contact list.
 
 1.Enter SMS APP,create a new meaasge.
 2.input a valid  recepient-->input a number(18658150713) in the text-->After sending succeed,long press the number,prompt two options,find it less a option "Call". Different from ergo.  --KO1
 3.From step2,select "create new contact",add the number you send in contact list.
 4.Then long press the number again,find the option "create new contact" is always exist. --KO2
 
 EXPECTED BEHAVIOUR:
 KO1:The option "Call" can exits.
 KO2:When the number add in contact list,long press the number again,the option "create new contact" is not exits. 
 
 ASSOCIATE SPECIFICATION:
 
 TEST PLAN REFERENCE:
 
 TOOLS AND PLATFORMS USED:
 
 USER IMPACT:
 Meidum
 
 REPRODUCING RATE:
 5/5
 
 For FT PR, Please list reference mobile's behavior:

Comment 1

[sync-1]

Clone from brother

Comment 2

[sync-1]

Attachment: 0727

Comment 3

[sync-1]

Clone from brother

Comment 4

[sync-1]

Attachment: ergo

Comment 5

[sync-1]

Clone from brother

Comment 6

[sync-1]

Attachment: 496345-logcat

Comment 7

[Rick Waldron [:rwaldron]]

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

Comment 8

[Julien Wajsberg [:julienw]]

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

I haven't really tested but I don't think you're adressing all the defects here.

1 - the "call" option should be here (that's what your patch does)
2 - the other options "create new contact" and "add to an existing contact" should not be displayed if the contact already exists

So basically we should have exactly the same behavior than what we do when we tap the header in a thread (except in that case we already know if we have a contact because we already looked it up before).

removing the review request for now.

Comment 9

[Rick Waldron [:rwaldron]]

All of the above addressed and then some. Turns out the email-link handling was totally wrong as well. 

In the meantime, Corey recommended that we add an :active state for the contextmenu buttons, so you can tell that your finger is interacting with the links (no such indicator exists). Ayman or Victoria, can you advise?

Comment 10

[ayman maat :maat]

(In reply to Rick Waldron from comment #9)
> In the meantime, Corey recommended that we add an :active state for the
> contextmenu buttons, so you can tell that your finger is interacting with
> the links (no such indicator exists). Ayman or Victoria, can you advise?

That's a visual design issue. Victoria is currently on holiday for a couple of weeks so NeedInfo to Silvia who is covering.

NeedInfo me again if you require my input.

Comment 11

[Silviaf]

Hello, my advise on the current issue, it could be good to let know that you are interacting with the links...lets use an active state that already exist for this kind of behaviour, so we dont need to create a new style. If such style doesnt exist, could you please send me a screenshot so i can see what state could we use. (my email is [e-mail])

Comment 12

[ayman maat :maat]

(In reply to Silviaf from comment #11)
> Hello, my advise on the current issue, it could be good to let know that you
> are interacting with the links...lets use an active state that already exist
> for this kind of behaviour, so we dont need to create a new style. If such
> style doesnt exist, could you please send me a screenshot so i can see what
> state could we use. (my email is [e-mail])

Cool. Could we make sure that all screenshots and solutions are passed through the bug please so we can keep track of them. 

Thanks :)

Comment 13

[Silviaf]

Yes of course, i just gave you guys my email so you have it, not to fix anything through email :)

Comment 14

[Rick Waldron [:rwaldron]]

Silvia, 

Are the :active rules from the browser adequate? See attached screenshot

Comment 15

[Rick Waldron [:rwaldron]]

Attachment: 899003-active.png

Comment 16

[Rick Waldron [:rwaldron]]

Silvia, 

After discussion with devs working on other apps and learning that they do not have :active for any links, we're going to punt on that aspect for this ticket and file a new one to specifically address it.

Comment 17

[Corey Frang [:gnarf]]

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

r=me

Comment 18

[Rick Waldron [:rwaldron]]

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

Still requesting leo+ because this fixes the completely incorrect email context menu (replaces with behaviour defined here https://bug899003.bugzilla.mozilla.org/attachment.cgi?id=782474)

Comment 19

[Silviaf]

Ok Rick, probably its for the best, seeing that it is more complex that we thought. Thanks

Comment 20

[Andrew Overholt [:overholt]]

(In reply to Rick Waldron from comment #18)
> Landed
> https://github.com/mozilla-b2g/gaia/commit/
> 546a8bf0004be4ac9dbcea0b7f6f407d53219406
> 
> Still requesting leo+ because this fixes the completely incorrect email
> context menu

Are you saying that without taking this, adding an email address to a contact (bug 838004) will be broken in 1.1?

Comment 21

[Julien Wajsberg [:julienw]]

I think this works for adding to a contact (needs to be verified) but for sure you'll have the "call" action on an email, which is wrong :-)

Comment 22

[Rick Waldron [:rwaldron]]

(In reply to Andrew Overholt [:overholt] from comment #20)
> (In reply to Rick Waldron from comment #18)
> > Landed
> > https://github.com/mozilla-b2g/gaia/commit/
> > 546a8bf0004be4ac9dbcea0b7f6f407d53219406
> > 
> > Still requesting leo+ because this fixes the completely incorrect email
> > context menu
> 
> Are you saying that without taking this, adding an email address to a
> contact (bug 838004) will be broken in 1.1?

I made a mistake in reading the spec and followed page 6 for both click and contextmenu on email addresses in the message body.

Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=900539

Comment 23

[Andrew Overholt [:overholt]]

(In reply to Rick Waldron from comment #22)
> (In reply to Andrew Overholt [:overholt] from comment #20)
> > (In reply to Rick Waldron from comment #18)
> > > Landed
> > > https://github.com/mozilla-b2g/gaia/commit/
> > > 546a8bf0004be4ac9dbcea0b7f6f407d53219406
> > > 
> > > Still requesting leo+ because this fixes the completely incorrect email
> > > context menu
> > 
> > Are you saying that without taking this, adding an email address to a
> > contact (bug 838004) will be broken in 1.1?
> 
> I made a mistake in reading the spec and followed page 6 for both click and
> contextmenu on email addresses in the message body.
> 
> Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=900539

Thanks, I see what the problem is now.  I'm removing leo? here as setting it on bug 900539 is clearer (as you've done).

Comment 24

[Rick Waldron [:rwaldron]]

(In reply to Andrew Overholt [:overholt] from comment #23)
> (In reply to Rick Waldron from comment #22)
> > (In reply to Andrew Overholt [:overholt] from comment #20)
> > > (In reply to Rick Waldron from comment #18)
> > > > Landed
> > > > https://github.com/mozilla-b2g/gaia/commit/
> > > > 546a8bf0004be4ac9dbcea0b7f6f407d53219406
> > > > 
> > > > Still requesting leo+ because this fixes the completely incorrect email
> > > > context menu
> > > 
> > > Are you saying that without taking this, adding an email address to a
> > > contact (bug 838004) will be broken in 1.1?
> > 
> > I made a mistake in reading the spec and followed page 6 for both click and
> > contextmenu on email addresses in the message body.
> > 
> > Filed: https://bugzilla.mozilla.org/show_bug.cgi?id=900539
> 
> Thanks, I see what the problem is now.  I'm removing leo? here as setting it
> on bug 900539 is clearer (as you've done).

No, they should _both_ be leo+

Comment 25

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

Uplifted 546a8bf0004be4ac9dbcea0b7f6f407d53219406 to:
v1-train: a70bd4dde16a3cdc0307ea220db3ed3ae298db53

Comment 26

[Tim Guan-tin Chien [:timdream] (please needinfo)]

v1.1.0hd: a70bd4dde16a3cdc0307ea220db3ed3ae298db53

Comment 27

[dkumar]

Verified fixed, the issue no longer reproduces.
Environmental Variables
Build ID: 20130814041202
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/15813d776a69
Gaia: dd3959fa74e356a528daa76ffee14c2c728a4b56
Platform Version: 18.1

>When the contact is new long pressing the number gives the user “Call” option along with other three options.
>If the contact already exists in the contact list the user gets two options  which is “Call” and “Send message”

Comment 28

[buri.blff]

It works well in the new build.