Outdated compass icon for the middle click autoscroll

VERIFIED FIXED in Firefox 58

Status

()

defect
P1
normal
VERIFIED FIXED
2 years ago
2 years ago

People

(Reporter: utybodev, Assigned: dao)

Tracking

(Blocks 1 bug, {good-first-bug})

57 Branch
Firefox 58
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox57 wontfix, firefox58 verified)

Details

(Whiteboard: [reserve-photon-visual])

Attachments

(2 attachments, 2 obsolete attachments)

(Reporter)

Description

2 years ago
Posted image New Photon compass icon (obsolete) —
The middle click autoscroll compass icon does not respect the new Photon UI guidelines, and looks more of a native icon from Windows 7. As such, it does not respect Windows 8 or 10 icons style, nor does it respect the general Photon UI, and looks very out of place.

I have made a new icon, following the Photon guidelines(1) as well as the MDN SVG Cleanup guide(2). This compass can be used for all of the different compass types by simply disabling or enabling the "northsouth" and "westeast" components. Note that the icon has a light background to allow it to be seen even on dark backgrounds.

(This icon was made by me after reddit user u/Jop902 pointed out the outdated icon, and u/zbraniecki suggested I make a new one)


(1) http://design.firefox.com/photon/visual/icons.html
(2) https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/SVG_Guidelines

Updated

2 years ago
Whiteboard: [photon-visual][triage]
Thank you for your contribution Matthieu!

:Gijs set the [triage] whiteboard status which will result in someone from the UX team evaluating if the icon needs any finetuning.

Once this is settled, I'll help you write a patch to get it into Firefox :)
(Assignee)

Comment 2

2 years ago
Shorlander, does the middle-click / autoscroll image need an update? Does the attached one look good to you?
Flags: needinfo?(shorlander)
This looks great, thank you!

The only tweak I would suggest is matching the 45º angle of our other arrow heads: http://design.firefox.com/icons/viewer/#arrow
Flags: needinfo?(shorlander)
(Assignee)

Comment 4

2 years ago
Matthieu, do you want to update the icon as Stephen suggests?
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(utybodev)
Priority: -- → P4
Whiteboard: [photon-visual][triage] → [reserve-photon-visual]
(Reporter)

Comment 5

2 years ago
I have fixed the icon following Stephen's suggestion. Does this look better?
Attachment #8896240 - Attachment is obsolete: true
Flags: needinfo?(utybodev)
Flags: needinfo?(shorlander)
(In reply to Matthieu "utybo" S. from comment #5)
> Created attachment 8900870 [details]
> Compass icon with fixed arrows angles
> 
> I have fixed the icon following Stephen's suggestion. Does this look better?

Yes, thank you.
Flags: needinfo?(shorlander)
(Assignee)

Comment 7

2 years ago
Matthieu, would you also like to write the patch for integrating this icon?
Flags: needinfo?(utybodev)

Comment 8

2 years ago
Posted image compass-icon.svg (obsolete) —
Optimized the SVG further and used context-fill.

Comment 9

2 years ago
(In reply to Dão Gottwald [::dao] from comment #7)
> Matthieu, would you also like to write the patch for integrating this icon?

This search should help: https://dxr.mozilla.org/mozilla-central/search?q=autoscroll.png&redirect=false
(Assignee)

Comment 10

2 years ago
(In reply to Tim Nguyen :ntim from comment #8)
> Created attachment 8901260 [details]
> compass-icon.svg
> 
> Optimized the SVG further and used context-fill.

Why would we use context-fill here?

Comment 11

2 years ago
(In reply to Dão Gottwald [::dao] from comment #10)
> (In reply to Tim Nguyen :ntim from comment #8)
> > Created attachment 8901260 [details]
> > compass-icon.svg
> > 
> > Optimized the SVG further and used context-fill.
> 
> Why would we use context-fill here?

The original PNG has different states, so I thought it may be useful here.

Comment 12

2 years ago
If it isn't, it could easily be removed anyway.
(Assignee)

Updated

2 years ago
Attachment #8901260 - Attachment is obsolete: true
(Reporter)

Comment 13

2 years ago
(In reply to Dão Gottwald [::dao] from comment #7)
> Matthieu, would you also like to write the patch for integrating this icon?

I think I would prefer having someone else qualified write the patch, as I have no idea of how to do it and I do not feel comfortable with playing around with Firefox sources. Sorry :/
Flags: needinfo?(utybodev) → needinfo?(dao+bmo)
(Assignee)

Comment 14

2 years ago
No problem, we can take care of this. Thanks for the image!
Flags: needinfo?(dao+bmo)
:dao - what's needed in this bug? Is it an easy-first-bug?
Flags: needinfo?(dao+bmo)
(Assignee)

Comment 16

2 years ago
We basically just need to update the references to autoscroll.png. It's not very heard but also not super trivial since autscroll.png has a weird format containing multiple icons.
Flags: needinfo?(dao+bmo)
Ok. So the next step is to generate the updated autoscroll.png with the new icon.

Here's the current one:

 * Windows: http://searchfox.org/mozilla-central/source/toolkit/themes/windows/global/icons/autoscroll.png
 * Mac: http://searchfox.org/mozilla-central/source/toolkit/themes/osx/global/icons/autoscroll.png
 * Linux: http://searchfox.org/mozilla-central/source/toolkit/themes/linux/global/icons/autoscroll.png

It seems to me that Windows uses the right column for XP_WIN (Windows XP) and left column for others.

The Mac and Linux only use the right column.

:shorlander - do you want per-platform icons for the new one? If so, can you instruct how to alter them?
Flags: needinfo?(shorlander)
(Assignee)

Comment 18

2 years ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #17)
> Ok. So the next step is to generate the updated autoscroll.png with the new
> icon.

No, we should update the CSS to reference the new SVG icon.
> No, we should update the CSS to reference the new SVG icon.

Ok.

But we need three versions of the icon - for horizontal, vertical and both directions scrolling, right?
Tim, I've seen your amazing work with icons all around! Congrats!

Is there any chance you'd be interested in helping with this bug? :)
Flags: needinfo?(ntim.bugs)
(Assignee)

Comment 21

2 years ago
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #19)
> > No, we should update the CSS to reference the new SVG icon.
> 
> Ok.
> 
> But we need three versions of the icon - for horizontal, vertical and both
> directions scrolling, right?

Yep, they're easy to produce from the attached SVG since Matthieu already grouped the "northsouth" and "westeast" paths.

Comment 22

2 years ago
I only use my trackpad daily, sorry.
Flags: needinfo?(ntim.bugs)
Flags: qe-verify?
(Assignee)

Updated

2 years ago
Flags: qe-verify? → qe-verify+
QA Contact: ovidiu.boca

Comment 23

2 years ago
If this is a good first bug, I would like to try helping with it as long as someone can point me in the right direction. The concept seems simple but I haven't worked with SVG files before.
(In reply to Zibi Braniecki [:gandalf][:zibi] from comment #17)
> :shorlander - do you want per-platform icons for the new one? If so, can you
> instruct how to alter them?

No, I don't think there is anything platform specific we need to do here.
Flags: needinfo?(shorlander)
(Assignee)

Comment 25

2 years ago
gandalf, still want to mentor this? There's interest in comment 23.
Flags: needinfo?(gandalf)
I did my best to collect all the relevant information from the stakeholders.

I think at this point all that has to be done is:

 * generate the correct SVG file
 * generate the correct CSS to switch between states
 * place the SVG in the tree and update CSS code

I've never worked on SVG+CSS so I'm not sure how those two steps should be done, but I can help with the third.
Flags: needinfo?(gandalf)
(Assignee)

Updated

2 years ago
Mentor: gandalf
(Assignee)

Updated

2 years ago
Assignee: nobody → dao+bmo
Status: NEW → ASSIGNED
Priority: P4 → P1
Comment hidden (mozreview-request)

Comment 28

2 years ago
mozreview-review
Comment on attachment 8915941 [details]
Bug 1389462 - Update autoscroll icon.

https://reviewboard.mozilla.org/r/187208/#review192220

Wasnt able to test due to lack of mouse, but codewise the patch looks simple and correct, cheers
Attachment #8915941 - Flags: review?(dharvey) → review+

Comment 29

2 years ago
Pushed by dgottwald@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/5b41d7a0fe73
Update autoscroll icon. r=daleharvey
https://hg.mozilla.org/mozilla-central/rev/5b41d7a0fe73
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
(Assignee)

Comment 31

2 years ago
Not going to try to uplift this to 57 -- let's have this ride the 58 release train.
Thank you for your contribution Matthieu! :)

Comment 33

2 years ago
I don't think this icon matches the default theme and cursors on Windows 7. Is it appropriate to add it to all platforms even if it doesn't match?

Right now it looks seriously out of place on my browser.
(Reporter)

Comment 34

2 years ago
Looks as if this is getting loads of criticism from r/firefox, you guys may want to have a look at it


https://www.reddit.com/r/firefox/comments/752s7x/thanks_to_mozilla_for_changing_the_middle_click/

Comment 35

2 years ago
IMHO this isn’t a UI element that needs to be “Photonized” to that extreme: even under Windows 10, the overly thick lines look quite ludicrous. It might look alright for high-contrast themes, though.

Updated

2 years ago
Depends on: 1406920

Comment 36

2 years ago
(In reply to Matthieu "utybo" S. from comment #34)
> Looks as if this is getting loads of criticism from r/firefox, you guys may
> want to have a look at it
> 
> 
> https://www.reddit.com/r/firefox/comments/752s7x/
> thanks_to_mozilla_for_changing_the_middle_click/

In case it helps, my intuitive expectation is that, unless everything including the default cursor has been re-themed (eg. in a game expected to run full-screen), all cursors are "OS elements" and, thus, should match the OS's design language rather than the application's.

(Or, to put it another way, consistency between cursors is paramount,)
Verified fixed using the latest Nightly (2017-10-13) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Status: RESOLVED → VERIFIED
Flags: qe-verify+
Depends on: 1410498
You need to log in before you can comment on or make changes to this bug.