gShareUtils should not live in browser.js
Categories
(Firefox :: Menus, task, P3)
Tracking
()
People
(Reporter: Gijs, Assigned: johnmax2468, Mentored, NeedInfo)
References
(Blocks 1 open bug)
Details
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.
Reporter | ||
Updated•7 months ago
|
Assignee | ||
Comment 1•7 months ago
|
||
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?
Reporter | ||
Comment 2•7 months ago
|
||
(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?
Description
•