Closed
Bug 1183659
Opened 9 years ago
Closed 9 years ago
Revise copy in "tab received" notification
Categories
(Firefox for Android Graveyard :: General, defect)
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
Reporter | ||
Comment 1•9 years ago
|
||
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)
Comment 2•9 years ago
|
||
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)
Comment 3•9 years ago
|
||
(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.
Comment 4•9 years ago
|
||
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.
Reporter | ||
Comment 5•9 years ago
|
||
(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)
Comment 6•9 years ago
|
||
(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)
Reporter | ||
Comment 7•9 years ago
|
||
(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.
Reporter | ||
Comment 8•9 years ago
|
||
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.
Assignee | ||
Comment 10•9 years ago
|
||
If this blocks bug 11480048, I guess it's on me.
Assignee: nobody → michael.l.comella
Assignee | ||
Comment 11•9 years ago
|
||
Anthony, can you summarize the updated copy? I'm having difficulty following the previous comments.
Flags: needinfo?(alam)
Reporter | ||
Comment 12•9 years ago
|
||
(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)
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(michael.l.comella)
Assignee | ||
Comment 13•9 years ago
|
||
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.
Assignee | ||
Comment 14•9 years ago
|
||
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.
Assignee | ||
Comment 15•9 years ago
|
||
(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).
Comment hidden (typo) |
Assignee | ||
Comment 17•9 years ago
|
||
(In reply to Michael Comella (:mcomella) from comment #16) > Patches will depend on bug 1208268. Patches will depend on patches in bug 1208268.
Assignee | ||
Comment 18•9 years ago
|
||
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)
Reporter | ||
Comment 19•9 years ago
|
||
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)
Assignee | ||
Comment 20•9 years ago
|
||
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)
Assignee | ||
Comment 21•9 years ago
|
||
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)
Assignee | ||
Comment 22•9 years ago
|
||
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.
Assignee | ||
Comment 23•9 years ago
|
||
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.
Reporter | ||
Comment 24•9 years ago
|
||
(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.
Assignee | ||
Comment 25•9 years ago
|
||
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)
Assignee | ||
Comment 26•9 years ago
|
||
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 27•9 years ago
|
||
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+
Assignee | ||
Comment 28•9 years ago
|
||
https://hg.mozilla.org/integration/fx-team/rev/dd0ccce3117a15dbe9071ce97380bc564775b374 Bug 1183659 - Update tab received notification copy. r=nalexander
Comment 29•9 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/dd0ccce3117a
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox45:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 45
Updated•3 years ago
|
Product: Firefox for Android → Firefox for Android Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•