776064 - Long press "tel:" links context menu options have unexpected behavior

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

From bug 775770:
A long press on a "tel:..." link has the following options:
*Open Link in New Tab
*Copy Phone Number (added with bug 775770)
*Share Link
*Bookmark Link
*Open With an App

New tab, share link, and bookmark link all use the phone number with the "tel:". Opening this link in Firefox causes the Phone app's numberpad to open. For this reason, I think we can remove "Open Link in New Tab" as it exhibits the same behavior as a short press except an extra tab is opened (a tab that you probably don't want).

I think sharing should use the number without the scheme (similar to email's bug 744662, though that's undecided) and bookmarking can be done away with (since it fits better to have an "Add to contacts" option anyway, as per bug 684380).

Comment 1

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Patch

Comment 2

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 644542 [details] [diff] [review]
Patch


>diff --git a/mobile/android/chrome/content/browser.js b/mobile/android/chrome/content/browser.js
>+    phoneNumberLinkContext: {

>+        if (uri) {
>+          return uri.schemeIs("tel");
>+        }

nit: we don't use {} for one-line blocks, unless the else (or if) block requires them

>diff --git a/mobile/android/locales/en-US/chrome/browser.properties b/mobile/android/locales/en-US/chrome/browser.properties

> contextmenu.copyEmailAddress=Copy Email Address
> contextmenu.copyPhoneNumber=Copy Phone Number
>+contextmenu.sharePhoneNumber=Share Phone Number

I am starting to not like using "Email Address" and "Phone Number" in these labels. Seems like we could just use "Copy" and "Share". The user knows what they tapped on.

The code is good, so r+, but let's get UX to weigh in on the labels.

Comment 3

[Mark Finkle (:mfinkle) (use needinfo?)]

Basically, with this bug and bug 744662, we won't offer a "Share" for phone numbers and email links. But we do offer special "Share Phone Number" and "Share Email Address" which remove the scheme of the link. 

I am wondering if we should just drop the "Phone Number" and "Email Address" and just show "Share" - which would still do the new special behavior (no scheme) but just use simpler text.

I don't know if "Copy" would follow the idea or not.

Technically, we could deal with any desired label changes in a new bug too.

Comment 4

[Ian Barlow (:ibarlow)]

I agree with Mark here -- the user knows what they have selected, so we don't need to specify the type of link they are sharing. Let's just keep it simple and consistent. 

As an aside, I'm happy to see that we're hiding the tel: and mailto: schemes when we share these links.

Comment 5

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

While I do prefer the brevity (both user-wise and code-wise), my one concern with dropping the "phone number"/"email address" labels is that one of these links can be placed on an image and it'd be ambiguous what you were sharing.

While not a phone number/email address, as an example, the infobox image on a Wikipedia page has both "Share Image" and "Share Link".

One possibility around this could be to see only "Share" and if there is more than one item to share, after clicking on "Share", another dialog will pop up and show an option to share the image or the link.

Comment 6

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Patch v2

(In reply to Mark Finkle (:mfinkle) from comment #2)
> Comment on attachment 644542 [details] [diff] [review]
> Patch
>
> nit: we don't use {} for one-line blocks, unless the else (or if) block
> requires them

Fixed nit. Moved r+. Need to move r?

Comment 7

[Ian Barlow (:ibarlow)]

(In reply to Michael Comella (:mcomella) from comment #5)

> One possibility around this could be to see only "Share" and if there is
> more than one item to share, after clicking on "Share", another dialog will
> pop up and show an option to share the image or the link.

Is there any way we can avoid a second menu in this case, possibly by providing those options, when available, in the first dialog box?

Comment 8

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

(In reply to Ian Barlow (:ibarlow) from comment #7)
> Is there any way we can avoid a second menu in this case, possibly by
> providing those options, when available, in the first dialog box?

This should be doable. I think it would be cleanest to put this change into a new bug as this patch (and the other similar one for mailto:) are mostly unrelated. Would you like this behavior to apply to "Copy" as well?

Similarly, we can reduce "Open Link in New Tab" and "Bookmark Link" to "Open in New Tab" and "Bookmark", but I think some ambiguity returns when there are other elements (ex: do you open the link or the image in the new tab?). It's possible to re-add the "Link" designation when there is another element present but I don't think the added code complexity is worth it for these two cases.

Comment 9

[Ian Barlow (:ibarlow)]

(In reply to Michael Comella (:mcomella) from comment #8)
> (In reply to Ian Barlow (:ibarlow) from comment #7)
> > Is there any way we can avoid a second menu in this case, possibly by
> > providing those options, when available, in the first dialog box?
> 
> This should be doable. I think it would be cleanest to put this change into
> a new bug as this patch (and the other similar one for mailto:) are mostly
> unrelated. Would you like this behavior to apply to "Copy" as well?
> 
> Similarly, we can reduce "Open Link in New Tab" and "Bookmark Link" to "Open
> in New Tab" and "Bookmark", but I think some ambiguity returns when there
> are other elements (ex: do you open the link or the image in the new tab?).
> It's possible to re-add the "Link" designation when there is another element
> present but I don't think the added code complexity is worth it for these
> two cases.

I'm not entirely clear on what you're asking me. You might have to draw me a picture. In a new bug, if necessary :)

Comment 10

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Surprisingly not bitrotted.

Comment 11

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

Attachment: Patch v3

Moved r+. Minor indentation fix.

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Green on Try.
https://tbpl.mozilla.org/?tree=Try&rev=2225f75a2f66

https://hg.mozilla.org/integration/mozilla-inbound/rev/02b6677f5f8d

Comment 13

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/02b6677f5f8d

Comment 14

[Ioana Chiorean]

Build ID: 17.0a2 (2012-10-08)Aurora Channel
          18.0 (2012-10-08) Nightly Channel
          19.0 (2012-10-09) Nightly Channel
Device: Samsung Galaxy Nexus
OS: Android 4.1

Tested with http://dump.lassey.us/proto.html - Context menu according to phone numbers. The "share link, bookmark link" are not available /not displayed.
Marking bug as verify fixed.