Closed Bug 1183659 Opened 9 years ago Closed 9 years ago

Revise copy in "tab received" notification

Categories

(Firefox for Android Graveyard :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox45 fixed)

RESOLVED FIXED
Firefox 45
Tracking Status
firefox45 --- fixed

People

(Reporter: antlam, Assigned: mcomella, Mentored)

References

Details

Attachments

(1 file)

From bug 783901:

I'd like to update the copy to both follow the guidelines set by the platform as well as unify with our other notifications a bit more.

Showing the device name rather than the page title makes it more glance-able and if you think of this notification on a platform such as a watch, a page title isn't terribly useful.

New tab from *device name
*link

Later on, we can also unify the notification like so:

New tabs from *device name
# tabs received
Can we update the copy on this notification? 

New Tab: $title \n $url isn't as glance-able as it could be and Android's system notifications are designed to not overload the users' attention. 

I think what's most important in this case is the device that sent it, and the link is sort of icing that can give some context in case the user forgets, or there's an extended period of disconnect between sending and receiving. Either way, Google provides a subtitle which is not that intrusive or clutters the UI so i figured we could take advantage of it.

I'm ok with hiding the page title because while neither the URL nor the Title of the page is perfect, at least the TLD isn't going to get truncated in most cases so a user can still see that part. Likewise, since this is just icing, we can use that to show a more general "# tabs received" when more than 1 tab has been received.
Flags: needinfo?(rnewman)
This is doable. One of the arguments to displayURI is the GUID of the sending client. We can join that against the clients table to get the description of the sender.

Notably this can fail, so we need fallback logic/strings for the case when we don't know who sent us the tab…

Anthony: you should also consider that the string will look something like

  New tab from Richard's Nightly on Windows 7

Do you want any punctuation in there?
Flags: needinfo?(rnewman)
(In reply to Richard Newman [:rnewman] from comment #2)

> Anthony: you should also consider that the string will look something like
> 
>   New tab from Richard's Nightly on Windows 7
> 
> Do you want any punctuation in there?

L10N might be weird if we try to make it read like a sentence.
It would need to be a substitution into a string, not a concat, but yes, even so. That's why we did "New Tab: " in the first place.
(In reply to Richard Newman [:rnewman] from comment #2)
> Anthony: you should also consider that the string will look something like
> 
>   New tab from Richard's Nightly on Windows 7
> 
> Do you want any punctuation in there?

Notifications on the platform tend to steer away from punctuation I find. I think that's the right decision for us too. 

(In reply to Mark Finkle (:mfinkle) from comment #3)
> (In reply to Richard Newman [:rnewman] from comment #2)
> 
> > Anthony: you should also consider that the string will look something like
> > 
> >   New tab from Richard's Nightly on Windows 7
> > 
> > Do you want any punctuation in there?
> 
> L10N might be weird if we try to make it read like a sentence.

(In reply to Richard Newman [:rnewman] from comment #4)
> It would need to be a substitution into a string, not a concat, but yes,
> even so. That's why we did "New Tab: " in the first place.

Fair point. I think that's also why these titles are often just as brief as "Subject" or  "App name". 

In light of these, maybe it's best to simply show the Device name in the title.
Flags: needinfo?(rnewman)
Flags: needinfo?(mark.finkle)
(In reply to Anthony Lam (:antlam) from comment #5)

> In light of these, maybe it's best to simply show the Device name in the
> title.

WFM… if you think the icon is descriptive enough? :)

But you still need to figure out a string for when the device name isn't known.
Mentor: rnewman, alam
Flags: needinfo?(rnewman)
Flags: needinfo?(mark.finkle)
(In reply to Richard Newman [:rnewman] from comment #6)
> (In reply to Anthony Lam (:antlam) from comment #5)
> 
> > In light of these, maybe it's best to simply show the Device name in the
> > title.
> 
> WFM… if you think the icon is descriptive enough? :)
> 
> But you still need to figure out a string for when the device name isn't
> known.

"Firefox" :D

It's what we already use for Tab queue and it follows the platform guidelines.

Otherwise we could try "New tab in Firefox" but that could get kinda confusing when placed next to each other.
For transparency, I also thought about "New tab in Firefox", "Tab received" or even "New tab". But when placed next to the current other notifications (including Tab queue notifications), it created an awkward user experience. Also keep in mind this is the fallback too.

Firefox
2 tabs waiting

Tab received
http://facebook.com/user/16dnka7so

The above kinda works. Since we have the icon on the left. But starts to look weird and inconsistent , especially when the user sends multiple tabs. If the user receives multiple tabs, the subtitle will change to "# tabs received" and I find that was a bit repetitive to have a notification that would effectively say:

Tab received
2 tabs received 

or

New tabs in Firefox
2 tabs received

So I think all things considered, this fallback would be our best option:

Firefox
5 tabs waiting

Firefox
2 tabs received

In this case, at least there's a grouping that the user can immediately relate to. Firefox has something for me. First, 5 tabs are waiting, next, 2 tabs are received. I think this would be better than having the user differentiate between "Firefox" and "New tab in Firefox" because that's a confusing thing to think about since they're all "tabs". 

(non fallback/ common case would be)

Firefox
5 tabs waiting

Device's Name
2 tabs received

In the case that the device name is in the title, the difference feels more immediately obvious to me so I'm not too worried.

Also, uniting these notifications in the future will be a topic of discussion and it'd be quite a bit easier to merge the two if they were more similar.
Something to clean up before moving the Shareplane
Blocks: 1140048
If this blocks bug 11480048, I guess it's on me.
Assignee: nobody → michael.l.comella
Anthony, can you summarize the updated copy? I'm having difficulty following the previous comments.
Flags: needinfo?(alam)
(In reply to Michael Comella (:mcomella) from comment #11)
> Anthony, can you summarize the updated copy? I'm having difficulty following
> the previous comments.

I can try! ...

When one link is received
---
Title: $DeviceName
Subtitle: $URL

When more than one link received
---
Title: $DeviceName
Subtitle: # tabs received

When $DeviceName is unavailable
---
Title: Firefox (or Nightly, Firefox Beta, etc)
Flags: needinfo?(alam) → needinfo?(michael.l.comella)
Flags: needinfo?(michael.l.comella)
On IRC, Nick & I discussed the possibility of decoupling the backend Sync code from the notifications display but it's too much yak shaving – I filed bug 1208268.
CommandProcessor.displayURI is static and we need to maintain state of how many notifications are shown so it'd be good to have an instance (e.g. of BroadcastReceiver!) – I'm going to look into bug 1208268 again.
(In reply to Michael Comella (:mcomella) from comment #14)
> CommandProcessor.displayURI is static and we need to maintain state of how
> many notifications are shown so it'd be good to have an instance (e.g. of
> BroadcastReceiver!) – I'm going to look into bug 1208268 again.

Turns out the BroadcastReceiver state is not maintained through separate invocations – looks like this information would have to be stored in static storage (e.g. shared prefs).
(In reply to Michael Comella (:mcomella) from comment #16)
> Patches will depend on bug 1208268.

Patches will depend on patches in bug 1208268.
Anthony, is $DeviceName in comment 12 the name of the device that received the tab or the one that sent the tab? In the latter, should each sending device have a separate notification or should we aggregate under "Firefox/Nightly/etc." at that point?
Flags: needinfo?(alam)
Senders :)

I think we can leave them separate for now (unless separating them is much harder). I want to take a look at all the notifications together to create some logic around when and how we merge these things. 

But that should probably be a different bug for scope.
Flags: needinfo?(alam)
Aggregating the notifications is harder than expected given our current interaction style. We open a generic Intent when opening the tab received notification and it can only hold a single uri. We can add more uris to an Intent but a generic application will not support reading them – we can only make Firefox do that. afaik we use generic Intents so we can share a link to a device and have it open directly in an app (e.g. open google maps links in Google Maps).

Solutions:
  * Create a custom Android chooser dialog that will let us choose the application to open and then proceed to call the Activity once for each tab we received.
  * Have aggregated notifications automatically open Firefox, removing the user choice. For consistency, a single received tab notification should probably also open Firefox.

Anthony, thoughts?
Flags: needinfo?(alam)
Spoke irl: we should open the links directly in firefox & aggregate them.

However, this may not be necessary: I just noticed the notification grouping APIs so this may solve the problem (before I was planning on aggregating them myself).
Flags: needinfo?(alam)
Nick gave me tips on how to get the device name:

14:16 <nalexander> mcomella: table "clients" has NAME and GUID.
14:17 <nalexander> mcomella: in browers DB.
14:17 <nalexander> browser DB.
14:17 <nalexander> mcomella: you can use the existing code to extract the entire DB, or play with it to extract by client GUID.
14:17 <nalexander> mcomella: I think that's well supported.
via IRC:

09:16 <@mfinkle> mcomella, antlam: mpopova and i were in a meeting yesterday where it came up that "send tab to device" style continuity would benefit from not always opening in fennec

Which means we we're going to hold aggregating sent tabs for now. It breaks Android convention not to aggregate but I find I don't send huge numbers of tabs anyway (unlike a messenger client) so I think it'll be find.

So for now, we just update the copy to the one in comment 12.
(In reply to Anthony Lam (:antlam) from comment #12)
> (In reply to Michael Comella (:mcomella) from comment #11)
> > Anthony, can you summarize the updated copy? I'm having difficulty following
> > the previous comments.
> 
> I can try! ...
> 
> When one link is received
> ---
> Title: $DeviceName
> Subtitle: $URL
>
> When $DeviceName is unavailable
> ---
> Title: Firefox (or Nightly, Firefox Beta, etc)

Talked to mcomella on IRC (commenting mostly for my own organization) 

This will be the update, and no merging will happen ATM.
Bug 1183659 - Update tab received notification copy. r=nalexander

The new format is:
  $DeviceName
  $url

Multiple received tab notifications are not aggregated.
Attachment #8683290 - Flags: review?(nalexander)
Comment on attachment 8683290 [details]
MozReview Request: Bug 1183659 - Update tab received notification copy. r=nalexander

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/24255/diff/1-2/
Comment on attachment 8683290 [details]
MozReview Request: Bug 1183659 - Update tab received notification copy. r=nalexander

https://reviewboard.mozilla.org/r/24255/#review21745

nits, and rebasing needed, but roll on.

::: mobile/android/base/sync/TabReceivedService.java:37
(Diff revision 2)
> +    public static final String EXTRA_CLIENT_ID = "org.mozilla.gecko.extra.CLIENT_ID";

`CLIENT_GUID` would be clearer.

::: mobile/android/base/sync/TabReceivedService.java:38
(Diff revision 2)
>      public static final String EXTRA_TITLE = "org.mozilla.gecko.extra.TITLE";

This will want rebasing on recent changes.

::: mobile/android/base/sync/TabReceivedService.java:87
(Diff revision 2)
> +     * @return the client's name from the clients table, if possible, else the brand name.

Note the threading restriction -- this needs to be on a background thread.

::: mobile/android/base/sync/TabReceivedService.java:89
(Diff revision 2)
> +    private String getNotificationTitle(@Nullable final String clientGuid) {

nit: `clientGUID`.
Attachment #8683290 - Flags: review?(nalexander) → review+
https://hg.mozilla.org/mozilla-central/rev/dd0ccce3117a
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: