Open Bug 1882774 Opened 8 months ago Updated 7 months ago

gShareUtils should not live in browser.js

Categories

(Firefox :: Menus, task, P3)

Desktop
All
task

Tracking

()

People

(Reporter: Gijs, Assigned: johnmax2468, Mentored, NeedInfo)

References

(Blocks 1 open bug)

Details

https://searchfox.org/mozilla-central/rev/9cd4ea81e27db6b767f1d9bbbcf47da238dd64fa/browser/base/content/browser.js#4709

The only consumers of this object are direct from menu popupshowing callers, so this could be in a sys.mjs module that is lazily loaded.

Severity: -- → N/A
Type: defect → task
Component: General → Menus
Priority: -- → P3
Mentor: gijskruitbosch+bugs

Hey, I decided to follow your recommendation and I would like to work on this issue. Anything I should be aware about for this refactor?

Flags: needinfo?(gijskruitbosch+bugs)

(In reply to Yi Xiong Wong from comment #1)

Hey, I decided to follow your recommendation and I would like to work on this issue. Anything I should be aware about for this refactor?

Hello - excellent! I'm honestly not 100% sure what you need to know. In case it's helpful: the share options are different on macOS and Windows (not sure what OS you're developing on) and so the menus behave differently in both cases. There are automated tests for both that you can run locally:

https://searchfox.org/mozilla-central/source/browser/base/content/test/menubar/browser_file_share.js (macOS)
https://searchfox.org/mozilla-central/source/browser/base/content/test/contextMenu/browser_contextmenu_share_macosx.js (macOS)
https://searchfox.org/mozilla-central/source/browser/base/content/test/contextMenu/browser_contextmenu_share_win.js (Windows)

It looks like at least one of the macOS tests would need to be updated if you move the sharing service ref (see below).

I think we probably just want a module (probably something like browser/modules/ShareUtils.sys.mjs), move the object there, and have consumers pass in a document or menuitem reference as needed (which it looks like they already do). Looks like the code inside the object also borrows the sharing service off of gBrowser (defined here) and that lazy getter should probably just move into the new module as well as there do not appear to be other consumers of the property.

Let me know if that helps / if there are other bits you have questions about?

Assignee: nobody → johnmax2468
Flags: needinfo?(gijskruitbosch+bugs) → needinfo?(johnmax2468)
You need to log in before you can comment on or make changes to this bug.