Replace placeholder Proton context menu navigation icons with final ones
Categories
(Firefox :: Menus, task, P1)
Tracking
()
People
(Reporter: mhowell, Assigned: mconley)
References
(Blocks 2 open bugs)
Details
(Whiteboard: [proton-context-menus] [proton-icons][proton-uplift])
Attachments
(1 file)
(deleted),
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta+
|
Details |
The final icons are not available as I write this, but the ones that bug 1682522 landed (in {browser,toolkit}/themes/shared/icons/menus/
) are probably not going to be them.
Reporter | ||
Updated•4 years ago
|
Assignee | ||
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Updated•4 years ago
|
Comment 2•4 years ago
|
||
We have a https://docs.google.com/document/d/114gLvaDoZpYnhtK_pPGgbMizphBbjrKp23_YHmWHHLE which provides links to where to find the new icon assets, suggested procedure and tracking spreadsheet for icon updates.
Note the stop/reload icons are being updated in bug 1702281, as they need to land at the same time as the updated reload-to-stop / stop-to-reload animations.
Comment 3•4 years ago
|
||
Will this address the horizontal icon placement in the context menu (they're currently placed too much on the right on Windows) or should I file a separate bug? (or maybe bug 1704275 will?)
Comment 4•4 years ago
|
||
(In reply to Mike Hommey [:glandium] from comment #3)
Will this address the horizontal icon placement in the context menu (they're currently placed too much on the right on Windows)
I can only guess at what this means without a screenshot, but it sounds like you have disabled proton context menus or are on an old build and are running into bug 1704618. Or perhaps you're talking about the bookmarks star being cut off, which will be fixed by this bug. But either way, without a screenshot, not sure what you mean.
Comment 5•4 years ago
|
||
(In reply to :Gijs (he/him) from comment #4)
(In reply to Mike Hommey [:glandium] from comment #3)
Will this address the horizontal icon placement in the context menu (they're currently placed too much on the right on Windows)
I can only guess at what this means without a screenshot, but it sounds like you have disabled proton context menus or are on an old build and are running into bug 1704618. Or perhaps you're talking about the bookmarks star being cut off, which will be fixed by this bug. But either way, without a screenshot, not sure what you mean.
I think this is now bug 1704772, so clearing needinfo.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 6•4 years ago
|
||
The bookmark icon is being taken care of in bug 1691993. This bug will take care of:
- browser/themes/shared/icons/menus/reload.svg (reload.svg)
- browser/themes/shared/icons/menus/back.svg (back.svg)
- browser/themes/shared/icons/menus/forward.svg (forward.svg)
Comment 7•4 years ago
|
||
(In reply to Mike Conley (:mconley) (:⚙️) (Catching up on needinfos) from comment #6)
The bookmark icon is being taken care of in bug 1691993. This bug will take care of:
- browser/themes/shared/icons/menus/reload.svg (reload.svg)
- browser/themes/shared/icons/menus/back.svg (back.svg)
- browser/themes/shared/icons/menus/forward.svg (forward.svg)
I think I told you this in matrix, but for completeness on this bug: These icons should just match the toolbar - we should be able to remove the additional menu icons and the CSS at https://searchfox.org/mozilla-central/source/browser/themes/windows/browser.css#861-873 , as then we'll fall back on the icons specified at https://searchfox.org/mozilla-central/rev/759872688df15a5d6ab305ffe39d90450590bfec/browser/themes/shared/contextmenu.inc.css#23-46 . As you said, the bookmark icon is being replaced in 1691993, and the reload one in bug 1702281. We might not want to land a patch for this bug until both 1702281 and 1691993 are on autoland, as verification of the fix will be confusing if the right icons aren't yet in place.
Updated•4 years ago
|
Assignee | ||
Comment 8•4 years ago
|
||
Comment 10•4 years ago
|
||
Comment 11•4 years ago
|
||
bugherder |
Assignee | ||
Comment 12•4 years ago
|
||
Comment on attachment 9215830 [details]
Bug 1692088 - Use the toolbar icons for back, forward, stop and reload in the Windows 10 context menu. r?mhowell
Beta/Release Uplift Approval Request
- User impact if declined: Required for MR1 / Proton
- Is this code covered by automated tests?: Yes
- Has the fix been verified in Nightly?: Yes
- Needs manual test from QE?: No
- If yes, steps to reproduce:
- List of other uplifts needed: None
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): Removes some placeholder icons and is very well contained to just the context menu on Windows.
- String changes made/needed: None.
Updated•4 years ago
|
Comment 13•4 years ago
|
||
Comment on attachment 9215830 [details]
Bug 1692088 - Use the toolbar icons for back, forward, stop and reload in the Windows 10 context menu. r?mhowell
Approved for beta, thanks
Comment 14•4 years ago
|
||
bugherder uplift |
Description
•