324361 - Remove openApplication/openApplicationWithURI from the shell service, but keep a function on Mac to open the desktop background app

Status: RESOLVED FIXED

(Firefox :: Shell Integration, defect)

Description

[:Gavin Sharp [email: [e-mail]]]

openApplication(APPLICATION_MAIL) and openApplication(APPLICATION_NEWS) are no longer used, which means openApplication can be removed for GNOME/Win, and trimmed for Mac (it seems to have other unused targets, APPLICATION_KEYCHAIN_ACCESS, APPLICATION_NETWORK, but I'm guessing those are meant to be used in the future?). nsWindowsShellService's GetUnreadMailCount is also no longer used and can be removed.

I don't know whether removing these is necessarily a good idea - it would save on codesize, but I can imagine they may be commonly used by extensions. Ben mentioned it in bug 306453, so filing this for discussion.

Comment 1

[:Gavin Sharp [email: [e-mail]]]

Attachment: potential patch

This builds on Windows, but hasn't been tested. I moved openApplication to nsIMacShellService since it still had a APPLICATION_DESKTOP caller on the Mac. This seems like it has the potential to break a lot of extensions.

Comment 2

[:Gavin Sharp [email: [e-mail]]]

unreadlMailCount was removed in bug 693227

Comment 3

[Mark Banner (:standard8)]

I'm happy to mentor someone on this.

I think the final aims of the patch here should be:

• Remove openApplication and openApplicationWithURI from the shell service (and associated constants)
• Implement a openDesktopBackgroundApp function for nsIMacShellService that does the same as what openApplication(Ci.nsIMacShellService.APPLICATION_DESKTOP); does now.

Hints:

• Start by looking at the attached patch, mostly you should just need to reapply it to the latest code.
• Useful link for finding relevant code: https://searchfox.org/mozilla-central/search?q=openApplication&case=false&regexp=false&path=
• We no longer need the uuid changes just above the interface nsI... lines.
• The dummy implementations in browser_UITour_defaultBrowser.js can also be removed.
• setDesktopBackground.js will need to be changed to point to the new function.
• Please use Phabricator to submit patches: https://moz-conduit.readthedocs.io/en/latest/phabricator-user.html

Comment 4

[Shivam Singhal [ :championshuttler ]]

Hi @Mark I would like to work on it , can I take this up?

Thanks

Comment 5

[Aashna]

Hey,
I'm an outreachy 2019 applicant. Can I take up this issue and work on it?

Comment 6

[Mark Banner (:standard8)]

Assigning to Shivam as they asked first.

@Aashna, you can find possible other bugs at https://codetribute.mozilla.org/

Comment 7

[Mark Banner (:standard8)]

No more takers, so maybe it wasn't a good mentored bug for some reason or just not found. In any case, quick late Friday afternoon patch time.

Comment 8

[Mark Banner (:standard8)]

Attachment: Bug 324361 - Remove openApplication/openApplicationWithURI from the shell service as they aren't used. r?Mossop

Keeps a function for opening the Mac desktop preferences.

Comment 9

[Mark Banner (:standard8)]

Note, if you want to test this, right-click on an image and select "Set As Desktop Background", then hit the "Set Desktop Background" button, and it will change and let you open up the preferences.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=20a5bb8372569cea8cdc231ae5cfd5d038ff8fea

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f4da3ae7efa6
Remove openApplication/openApplicationWithURI from the shell service as they aren't used. r=mossop

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/f4da3ae7efa6