Closed Bug 1692088 Opened 10 months ago Closed 8 months ago

Replace placeholder Proton context menu navigation icons with final ones

Categories

(Firefox :: Menus, task, P1)

Unspecified
Windows 10
task

Tracking

()

RESOLVED FIXED
90 Branch
Tracking Status
firefox89 --- fixed
firefox90 --- fixed

People

(Reporter: mhowell, Assigned: mconley)

References

(Blocks 2 open bugs)

Details

(Whiteboard: [proton-context-menus] [proton-icons][proton-uplift])

Attachments

(1 file)

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.

Whiteboard: [proton-context-menus]
Severity: -- → N/A
Priority: -- → P3
Blocks: proton-icons
Priority: P3 → P2
Whiteboard: [proton-context-menus] → [proton-context-menus] [priority:2a]

Marking as P1 given it's icon work

Priority: P2 → P1
Depends on: 1686527
Whiteboard: [proton-context-menus] [priority:2a] → [proton-context-menus] [proton-icons][priority:2a]
Whiteboard: [proton-context-menus] [proton-icons][priority:2a] → [proton-context-menus] [proton-icons]
Depends on: 1702281

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.

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?)

(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.

Flags: needinfo?(mh+mozilla)

(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.

Flags: needinfo?(mh+mozilla)
Assignee: nobody → mconley

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)

(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.

Depends on: 1691993
Status: NEW → ASSIGNED
Duplicate of this bug: 1705907
Pushed by mconley@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/c629d6579040
Use the toolbar icons for back, forward, stop and reload in the Windows 10 context menu. r=mhowell
Status: ASSIGNED → RESOLVED
Closed: 8 months ago
Resolution: --- → FIXED
Target Milestone: --- → 90 Branch

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.
Attachment #9215830 - Flags: approval-mozilla-beta?
Whiteboard: [proton-context-menus] [proton-icons] → [proton-context-menus] [proton-icons][proton-uplift]

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

Attachment #9215830 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
You need to log in before you can comment on or make changes to this bug.