Closed
Bug 1293467
Opened 8 years ago
Closed 8 years ago
send tab notification has no icon
Categories
(Firefox :: Sync, defect, P1)
Firefox
Sync
Tracking
()
VERIFIED
FIXED
Firefox 51
Tracking | Status | |
---|---|---|
firefox51 | --- | verified |
People
(Reporter: edwong, Assigned: eoger)
Details
(Whiteboard: [send-tab])
Attachments
(3 files)
setup two profiles signed into the same sync acct 1. on desktop 1 - context click on page and send tab to all devices 2. on desktop 2 - poke sync now button actual: you a notification with no image or icon
Reporter | ||
Comment 1•8 years ago
|
||
a Logo would spruce things up. Also the originating device name italicized or on a new line would provide more visual clarity.
Flags: needinfo?(rfeeley)
Whiteboard: [send-tab]
Comment 2•8 years ago
|
||
We did something similar for the "notifications upgraded" prompt: http://searchfox.org/mozilla-central/rev/d83f1528dbcb1e837d99cf5b6c36a90b03bf0e77/browser/components/nsBrowserGlue.js#2250-2253 Native notifications already use the app icon, and we specified the logo manually for XUL notifications.
Updated•8 years ago
|
Whiteboard: [send-tab] → [send-tab][triage]
Comment 3•8 years ago
|
||
We should use the Firefox (or Aurora, Nightly, etc) icon like we do on OS X.
Flags: needinfo?(rfeeley)
Reporter | ||
Updated•8 years ago
|
Assignee: nobody → eoger
Priority: -- → P1
Updated•8 years ago
|
Status: NEW → ASSIGNED
Comment 5•8 years ago
|
||
It looks a little big to me. Here are the guidelines that make it look like you can use the application icon, and not place it as a big image. https://msdn.microsoft.com/en-us/windows/uwp/controls-and-patterns/tiles-and-notifications-adaptive-interactive-toasts Here is a the dropbox example. http://cdn.arstechnica.net/wp-content/uploads/2016/01/3-Interactive-toast-notifs.png
Flags: needinfo?(rfeeley) → needinfo?(eoger)
Assignee | ||
Comment 6•8 years ago
|
||
To be honest, I realized while setting up Sync for my tests that we don't have any icon for any of the new notifications we display (sync starting, validation email sent etc). I think we should open a bug asking for a default icon, just like on OSX, in the Notifications component instead of trying to make an half-assed fix.
Flags: needinfo?(eoger) → needinfo?(ckarlof)
Comment 7•8 years ago
|
||
Kit, didn't you work on some of this code? Do you know why we don't get the default Firefox icon for Notifications on Windows?
Flags: needinfo?(ckarlof) → needinfo?(kcambridge)
Comment 8•8 years ago
|
||
(In reply to Chris Karlof [:ckarlof] from comment #7) > Kit, didn't you work on some of this code? Do you know why we don't get the > default Firefox icon for Notifications on Windows? The simple answer is that the designs for XUL notifications (https://mozilla.invisionapp.com/share/3A4A895XZ#/screens/104465680) didn't call for an application icon if one wasn't specified. I think we considered using it in place of the favicon, but I'm not certain. We don't control it on OS X, but, on Windows, it's up to us if we want a default. Let's file a bug in Toolkit :: Notifications, and we can discuss further. In the meantime, I think Edouard's patch is a good workaround. (In reply to Ryan Feeley [:rfeeley] from comment #5) > Here is a the dropbox example. > http://cdn.arstechnica.net/wp-content/uploads/2016/01/3-Interactive-toast- > notifs.png Ryan, it looks like Dropbox uses native Windows notifications, which we don't implement yet. We can, of course, and probably should, but that's a separate chunk of work. ;-)
Flags: needinfo?(kcambridge)
Comment 9•8 years ago
|
||
Ryan, do you have thoughts on using the oversize app icon vs. leaving it without an icon at all?
Flags: needinfo?(rfeeley)
Comment 10•8 years ago
|
||
Or using a smaller icon. :-)
Comment 11•8 years ago
|
||
We should use the smaller icon à la Dropbox example so that when we do move to native notifications, the transition is seamless.
Flags: needinfo?(rfeeley)
Updated•8 years ago
|
Whiteboard: [send-tab][triage] → [send-tab]
Comment hidden (mozreview-request) |
Comment 13•8 years ago
|
||
mozreview-review |
Comment on attachment 8784617 [details] Bug 1293467 - Explicitely show icon on tab received notification on Windows. https://reviewboard.mozilla.org/r/73974/#review71890
Attachment #8784617 -
Flags: review?(markh) → review+
Assignee | ||
Updated•8 years ago
|
Keywords: checkin-needed
Comment 14•8 years ago
|
||
Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/0f2ee5d1f19b Explicitely show icon on tab received notification on Windows. r=markh
Keywords: checkin-needed
Comment 15•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/0f2ee5d1f19b
Status: ASSIGNED → RESOLVED
Closed: 8 years ago
status-firefox51:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → Firefox 51
Updated•8 years ago
|
Flags: qe-verify+
Comment 16•7 years ago
|
||
Verified fixed on Windows 7 x64 using Firefox 51 Beta 9 (buildID: 20161219140923): send tab notification has icon.
Status: RESOLVED → VERIFIED
You need to log in
before you can comment on or make changes to this bug.
Description
•