Closed Bug 1482646 Opened Last year Closed Last year

Autoplay icon too small

Categories

(Firefox :: Site Identity and Permission Panels, defect)

defect
Not set

Tracking

()

VERIFIED FIXED
Firefox 63
Tracking Status
firefox64 --- verified

People

(Reporter: daleharvey, Assigned: daleharvey)

References

Details

Attachments

(5 files, 2 obsolete files)

Dao suggested the icon was too small @ https://bugzilla.mozilla.org/show_bug.cgi?id=1481289#c11
Hey Bryant, is this purposeful or do we want to change it here?
Assignee: nobody → dharvey
Flags: needinfo?(bmao)
(In reply to Dale Harvey (:daleharvey) from comment #1)
> Created attachment 8999381 [details]
> Screen Shot 2018-08-11 at 17.27.30.png
> 
> Hey Bryant, is this purposeful or do we want to change it here?

Hey Dale, sorry for the late reply. Can you tell me the size of the icon asset first? Is it 16x16?
Yup the icon canvas (and the space its in) is 16x16, the actual pixels that make up the circle icon are 12x12
Duplicate of this bug: 1483513
Blocks: 1461656
Forwarding needinfo from Bryant to Mark as he is working on this now, cheers
Flags: needinfo?(bmao) → needinfo?(mliang)
Hi Dale, the owner of photon icons, Bryan Bell, is working on a new icon for autoplay. Once that's done, I'll update the icon here and should be able to fix this problem.

Also related to this: https://bugzilla.mozilla.org/show_bug.cgi?id=1481289#c21
Flags: needinfo?(mliang)
I attached the updated icons here, let me know if it works, thanks!
(In reply to Mark Liang(:mark_liang) from comment #7)
> Created attachment 9003004 [details]
> update autoplay icon

The outer circle looks like a reload symbol to me. I'm having a hard time making sense of this. Can we simplify this, drop the circle and just have the "play" triangle?
Flags: needinfo?(mliang)
I actually like the circle, kind of suggesting that something is automatically running. Anyway I will let Bryan explain the ideas here. (Bryan is the design owner of the Photon icons.)
Flags: needinfo?(mliang) → needinfo?(bbell)
The design of the icon was intentional and went through several dozen revisions. The forward arrow-circle needs to stay.
Flags: needinfo?(bbell)
Bryan gave me a 32px version to use inside the doorhanger popup as well
Attachment #9004286 - Flags: review?(dao+bmo)
Attached image autoplay-32.svg
Hey Dale, here's the 32px version.
Comment on attachment 9004286 [details] [diff] [review]
0001-Bug-1482646-Update-icons-for-autoplay-media.-r-dao.patch

>--- a/browser/themes/shared/notification-icons.inc.css
>+++ b/browser/themes/shared/notification-icons.inc.css
>@@ -57,7 +57,10 @@
>   list-style-image: url(chrome://browser/skin/notification-icons/geo-blocked.svg);
> }
> 
>-.popup-notification-icon[popupid="autoplay-media"],
>+.popup-notification-icon[popupid="autoplay-media"] {
>+  list-style-image: url(chrome://browser/skin/notification-icons/autoplay-media-doorhanger.svg);
>+}
>+
> .autoplay-media-icon {
>   list-style-image: url(chrome://browser/skin/notification-icons/autoplay-media.svg);
> }

You put these rules in the middle of two geolocation rules that belong together. Please clean this up.

(In reply to Dale Harvey (:daleharvey) from comment #13)
> Created attachment 9004286 [details] [diff] [review]
> 0001-Bug-1482646-Update-icons-for-autoplay-media.-r-dao.patch
> 
> Bryan gave me a 32px version to use inside the doorhanger popup as well

It's an SVG, we can resize it. Is there a particular reason why that doesn't work well here? If so, please name the bigger icon "autplay-media-detailed" to be consistent with other icons.
Attachment #9004286 - Flags: review?(dao+bmo) → review-
Updated, cheers

The icons do render differently, https://imgur.com/a/JNd32Lc, the detailed one does look better @ 32px and unusable @ 16px
Attachment #9004286 - Attachment is obsolete: true
Attachment #9004531 - Flags: review?(dao+bmo)
Comment on attachment 9004531 [details] [diff] [review]
0001-Bug-1482646-Update-icons-for-autoplay-media.-r-dao.patch

>+.popup-notification-icon[popupid="autoplay-media"] {
>+  list-style-image: url(chrome://browser/skin/notification-icons/autoplay-media-detailed.svg);
>+}
>+
> .autoplay-media-icon {
>   list-style-image: url(chrome://browser/skin/notification-icons/autoplay-media.svg);
> }

nit: Put the .popup-notification-icon[popupid="autoplay-media"] rule after the .autoplay-media-icon rule to be consistent with other similar rules in this file.
Attachment #9004531 - Flags: review?(dao+bmo) → review+
Done thanks
Attachment #9004531 - Attachment is obsolete: true
Attachment #9004535 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/ea6c1463602c
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
QA Whiteboard: [good first verify]
I have reproduced this bug with Nightly 63.0a1(2018-08-11) on Windows 10, 64 bit!

The fix is now verified on Latest Nightly.

Build ID 	20181007100131
User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0) Gecko/20100101 Firefox/64.0


[bugday-20181003]
(In reply to Md.Majedul islam from comment #21)
> I have reproduced this bug with Nightly 63.0a1(2018-08-11) on Windows 10, 64
> bit!
> 
> The fix is now verified on Latest Nightly.
> 
> Build ID 	20181007100131
> User Agent 	Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:64.0)
> Gecko/20100101 Firefox/64.0
> 
> 
> [bugday-20181003]

Thank you for verifying! 

Updating the tracking flags accordingly as now nightly is Fx 64 and this has not landed in beta yet.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.