Closed Bug 888150 Opened 7 years ago Closed 7 years ago

[MMS][Regression] Header actions are not working properly.

Categories

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

ARM
Gonk (Firefox OS)
defect

Tracking

(blocking-b2g:leo+, b2g18 fixed, b2g-v1.1hd fixed)

VERIFIED FIXED
1.1 QE4 (15jul)
blocking-b2g leo+
Tracking Status
b2g18 --- fixed
b2g-v1.1hd --- fixed

People

(Reporter: borjasalguero, Assigned: rwaldron)

References

Details

(Keywords: regression, Whiteboard: [u=commsapps-user c=messaging p=0], [LeoVB+] )

Attachments

(1 file)

A lot of issues here....
STR1:
- Send a SMS to '123123123'
- See the thread
- Tap on the header
- See the list of options
- Tap on 'Call'
EXPECTED:
- You can call to '123123123' from the dialer
CURRENTLY:
- Nothing happens


STR2:
- Send a SMS to '123123123'
- See the thread
- Tap on the header
- See the list of options

EXPECTED:
- 'send message' should be hidden, because you are in your own thread
CURRENTLY:
- 'send messag is there

STR2:
- Send a SMS to '123123123'
- See the thread
- Tap on the header
- See the list of options

EXPECTED:
- 'send message' should be hidden, because you are in your own thread
CURRENTLY:
- 'send message' is there

STR3:
- Send a SMS to a contact 'Manolo'
- See the thread
- Tap on the header

EXPECTED:
- You will go directly to the dialer for calling 'Manolo'
CURRENTLY:
- You see wrong options, and you can not go to dialer directly.
This functionality is completely broken. Rick, I know you were working on this, could you take a look? Thanks
Assignee: nobody → waldron.rick
blocking-b2g: --- → leo?
Keywords: regression
Dietrich, this is a completely broken functionality, and a regression as well... leo+?
Flags: needinfo?(dietrich)
Im gonna fix the STR1:

 STR1:
 - Send a SMS to '123ABC'
 - See the thread
 - Tap on the header
 - See the list of options
 - Tap on 'Call'
 EXPECTED:
 - You can call to '123123123' from the dialer
 CURRENTLY:
 - Nothing happens
Ayman, I'd like to be able to go to the contact card when tapping on the thread header, as an additional action in the menu. Do you think it would be a good idea ?
Flags: needinfo?(aymanmaat)
(In reply to Julien Wajsberg [:julienw] from comment #4)
> Ayman, I'd like to be able to go to the contact card when tapping on the
> thread header, as an additional action in the menu. Do you think it would be
> a good idea ?

I see no reason why not, so long as this does not bring risk/extra work to the current build. If not we can defer to 1.2.

Please NeedInfo me to let me know the decision on this so i can update the v1.1 spec accordingly if necessary.
Flags: needinfo?(aymanmaat)
Flags: needinfo?(felash)
I really think this is a very small work, but I'll let Rick (the assignee) decides if it's worth it now or if we'll do it in 1.2.
Flags: needinfo?(felash)
Priority: -- → P1
Target Milestone: --- → 1.1 QE4 (15jul)
(In reply to Julien Wajsberg [:julienw] from comment #6)
> I really think this is a very small work, but I'll let Rick (the assignee)
> decides if it's worth it now or if we'll do it in 1.2.

ok Julien, referencing comment 4, 5 and 6 needinfo to Rick on this so we can either move forwards with it, or defer to v1.2
Flags: needinfo?(waldron.rick)
leo+ for STR1. In what bugs will the other STR be filed in, so that they can be nominated individually?
blocking-b2g: leo? → leo+
With:

b2g18@latest
gaia@master


(In reply to Borja Salguero [:borjasalguero] from comment #0)
> A lot of issues here....
> STR1:
> - Send a SMS to '123123123'
> - See the thread
> - Tap on the header
> - See the list of options
> - Tap on 'Call'
> EXPECTED:
> - You can call to '123123123' from the dialer
> CURRENTLY:
> - Nothing happens


This cannot be reproduced.

https://www.dropbox.com/sh/3lr5a5kepnns8z1/rydxqmpucg


> 
> 
> STR2:
> - Send a SMS to '123123123'
> - See the thread
> - Tap on the header
> - See the list of options
> 
> EXPECTED:
> - 'send message' should be hidden, because you are in your own thread
> CURRENTLY:
> - 'send messag is there


"Send Message" will be removed for single participant messages (will still appear in a _sent_ multi-participant message)

> 
> STR3:
> - Send a SMS to a contact 'Manolo'
> - See the thread
> - Tap on the header
> 
> EXPECTED:
> - You will go directly to the dialer for calling 'Manolo'
> CURRENTLY:
> - You see wrong options, and you can not go to dialer directly.

Seems strange to have this behavioral exception, but if that's the spec, then this will be updated.
Flags: needinfo?(waldron.rick)
(In reply to Julien Wajsberg [:julienw] from comment #4)
> Ayman, I'd like to be able to go to the contact card when tapping on the
> thread header, as an additional action in the menu. Do you think it would be
> a good idea ?

This should be easy enough.
(In reply to Rick Waldron from comment #10)
> (In reply to Julien Wajsberg [:julienw] from comment #4)
> > Ayman, I'd like to be able to go to the contact card when tapping on the
> > thread header, as an additional action in the menu. Do you think it would be
> > a good idea ?
> 
> This should be easy enough.

"should", but isn't. I may have missed something, but the Contacts app doesn't appear to have an activity for viewing a contact based on some parameter.
(In reply to Rick Waldron from comment #11)
> (In reply to Rick Waldron from comment #10)
> > (In reply to Julien Wajsberg [:julienw] from comment #4)
> > > Ayman, I'd like to be able to go to the contact card when tapping on the
> > > thread header, as an additional action in the menu. Do you think it would be
> > > a good idea ?
> > 
> > This should be easy enough.
> 
> "should", but isn't. I may have missed something, but the Contacts app
> doesn't appear to have an activity for viewing a contact based on some
> parameter.

This seems like a good idea.  If we won't be doing that in this bug, can we create bugs to track the feature request?  Is all we need "[Contacts] "view" activity for contact", and then "[SMS] allow "view" contact from header action menu" ?
(In reply to Corey Frang [:gnarf] [:gnarf37] from comment #12)
> (In reply to Rick Waldron from comment #11)
> > (In reply to Rick Waldron from comment #10)
> > > (In reply to Julien Wajsberg [:julienw] from comment #4)
> > > > Ayman, I'd like to be able to go to the contact card when tapping on the
> > > > thread header, as an additional action in the menu. Do you think it would be
> > > > a good idea ?
> > > 
> > > This should be easy enough.
> > 
> > "should", but isn't. I may have missed something, but the Contacts app
> > doesn't appear to have an activity for viewing a contact based on some
> > parameter.
> 
> This seems like a good idea.  If we won't be doing that in this bug, can we
> create bugs to track the feature request?  Is all we need "[Contacts] "view"
> activity for contact", and then "[SMS] allow "view" contact from header
> action menu" ?

I think the "open" activity does just that.

But I'll file another bug for this after all, because we need careful UX decision to be sure that everything feels ok, I don't want to rush for this.
Filed bug 890209 for this specific topic.
Whiteboard: [u=commsapps-user c=messaging p=0]
Rick, I will review this tomorrow morning. Sorry for the delay.
Flags: in-moztrap-
Comment on attachment 771001 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10771

Hi Rick,
I've been testing your patch and I found an issue here https://github.com/mozilla-b2g/gaia/pull/10771#issuecomment-20734398 . Please take a look, and once this will be fixed let me know and I will review this with priority. Thanks!
Attachment #771001 - Flags: review?(fbsc)
(In reply to Borja Salguero [:borjasalguero] from comment #17)
> Comment on attachment 771001 [details] [review]
> Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10771
> 
> Hi Rick,
> I've been testing your patch and I found an issue here
> https://github.com/mozilla-b2g/gaia/pull/10771#issuecomment-20734398 .
> Please take a look, and once this will be fixed let me know and I will
> review this with priority. Thanks!

Yes, you r+ed the code that broke this: https://github.com/mozilla-b2g/gaia/commit/8b41ee9a80c1d6b3613339e7388035d3fbcc575d#L2R227 

https://bugzilla.mozilla.org/show_bug.cgi?id=880624#c18
Borja, I've fixed all the regressions that bug 880624 created.
Attachment #771001 - Flags: review?(fbsc)
Attachment #771001 - Flags: review?(francisco.jordano)
Clear ni? for dietrich from comment 2. Already a leo+ bug.
Flags: needinfo?(dietrich)
Comment on attachment 771001 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10771

This is working as expected. R+ from my side! Waiting Francisco for merging this.
Attachment #771001 - Flags: review?(fbsc) → review+
Comment on attachment 771001 [details] [review]
Github Pull Request https://github.com/mozilla-b2g/gaia/pull/10771

r+

tried the use cases that were failing, now everything looking awesome.

Thanks a lot Rick!
Attachment #771001 - Flags: review?(francisco.jordano) → review+
Uplifted aaafec3c79e94041db7d22d3490f8d4916faeb3a to:
v1-train: 7fc1f9b4677909ac833817a550bbb21671f7a964
Whiteboard: [u=commsapps-user c=messaging p=0] → [u=commsapps-user c=messaging p=0], [LeoVB+]
v1.1.0hd: 7fc1f9b4677909ac833817a550bbb21671f7a964
Tested on Unagi device. 
Build:unagi-ICS.user.manifest.V1-train.Rel0.4.Sprint12.B-332.Gecko-f89d83b.Gaia-6221737
Build ID:20130730104759
Commercial RIL: AU 172

STEPS:
All includes in "Description"
(In reply to Borja Salguero [:borjasalguero] from comment #0)
> A lot of issues here....
> STR1:
> - Send a SMS to '123123123'
> - See the thread
> - Tap on the header
> - See the list of options
> - Tap on 'Call'
> EXPECTED:
> - You can call to '123123123' from the dialer
> CURRENTLY:
> - Nothing happens
> 
> 
> STR2:
> - Send a SMS to '123123123'
> - See the thread
> - Tap on the header
> - See the list of options
> 
> EXPECTED:
> - 'send message' should be hidden, because you are in your own thread
> CURRENTLY:
> - 'send messag is there
> 
> STR3:
> - Send a SMS to a contact 'Manolo'
> - See the thread
> - Tap on the header
> 
> EXPECTED:
> - You will go directly to the dialer for calling 'Manolo'
> CURRENTLY:
> - You see wrong options, and you can not go to dialer directly.

ALL ISSUES/TESTS WORK WELL, AS EXPECTED
Status: RESOLVED → VERIFIED
Attachment mime type: text/plain → text/x-github-pull-request
You need to log in before you can comment on or make changes to this bug.