Closed Bug 776064 Opened 7 years ago Closed 7 years ago

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

Categories

(Firefox for Android :: General, defect)

ARM
Android
defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 17

People

(Reporter: mcomella, Assigned: mcomella)

References

(Blocks 1 open bug)

Details

Attachments

(1 file, 2 obsolete files)

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).
Attached patch Patch (obsolete) — Splinter Review
Attachment #644542 - Flags: review?(mark.finkle)
Assignee: nobody → michael.l.comella
Status: NEW → ASSIGNED
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.
Attachment #644542 - Flags: review?(mark.finkle)
Attachment #644542 - Flags: review?(ibarlow)
Attachment #644542 - Flags: review+
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.
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.
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.
Attached patch Patch v2 (obsolete) — Splinter Review
(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?
Attachment #644542 - Attachment is obsolete: true
Attachment #644542 - Flags: review?(ibarlow)
Attachment #645084 - Flags: review+
(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?
(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.
(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 :)
Surprisingly not bitrotted.
Keywords: checkin-needed
Attached patch Patch v3Splinter Review
Moved r+. Minor indentation fix.
Attachment #645084 - Attachment is obsolete: true
Attachment #654294 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/02b6677f5f8d
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 17
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.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.