1901703 - unify firefox.desktop across tree

Status: RESOLVED FIXED

(Firefox Build System :: Third Party Packaging, defect, P3)

Description

[:gerard-majax]

As mentionned in https://bugzilla.mozilla.org/show_bug.cgi?id=1896374#c9 we have taskcluster/docker/firefox-snap/firefox.desktop and browser/components/shell/search-provider-files/firefox.desktop, we should look at unifying both

Comment 1

[:gerard-majax]

we will need to send PR against firefox-snap build as well

Comment 2

[:gerard-majax]

Differences when ignoring all localization:

$ diff -uw snap_firefox.desktop browser_firefox.desktop
--- snap_firefox.desktop        2024-07-08 13:30:19.714117010 +0200
+++ browser_firefox.desktop     2024-07-08 13:30:20.662140361 +0200
@@ -1,23 +1,25 @@
 [Desktop Entry]
 Version=1.0
-Name=Firefox Web Browser
-Comment=Browse the World Wide Web
+Name=Firefox
 GenericName=Web Browser
-Keywords=Internet;WWW;Browser;Web;Explorer
+Comment=Browse the Web
 Exec=firefox %u
+Icon=firefox
 Terminal=false
-X-MultipleArgs=false
 Type=Application
-Icon=/browser/chrome/icons/default/default128.png
-Categories=GNOME;GTK;Network;WebBrowser;
-MimeType=text/html;text/xml;application/xhtml+xml;application/xml;application/rss+xml;application/rdf+xml;image/gif;image/jpeg;image/png;x-scheme-handler/http;x-scheme-handler/https;x-scheme-handler/ftp;x-scheme-handler/chrome;video/webm;application/x-xpinstall;
+MimeType=text/html;text/xml;application/xhtml+xml;application/vnd.mozilla.xul+xml;text/mml;x-scheme-handler/http;x-scheme-handler/https;
 StartupNotify=true
-Actions=NewWindow;NewPrivateWindow;
+Categories=Network;WebBrowser;
+Keywords=web;browser;internet;
+Actions=new-window;new-private-window;
+DBusActivatable=true

-[Desktop Action NewWindow]
+X-Desktop-File-Install-Version=0.24
+
+[Desktop Action new-window]
 Name=Open a New Window
-Exec=firefox -new-window
+Exec=firefox --new-window %u

-[Desktop Action NewPrivateWindow]
+[Desktop Action new-private-window]
 Name=Open a New Private Window
-Exec=firefox -private-window
+Exec=firefox --private-window %u

Comment 3

[:gerard-majax]

Martin, I suspect you might be the person to review that. What do you think we merge those differences together? And what should be the one we prefer?

There are a few entries where we might not want to merge: DBusActivatable, X-MultipleArgs for example. Also the Exec on Snap do not have %u (I believe because this partly broken due to Snap sandboxing, although an URL works fine here)

Comment 4

[:gerard-majax]

Johan, you committed the file first, what is your opinion on the same items ?

Comment 5

[Sylvestre Ledru [:Sylvestre]]

maybe submit it to review? for easier feedback?

Comment 6

[Johan Lorenzo [:jlorenzo]]

What do you think we merge those differences together?

Let's do it! 👍 No need to maintain this duplication anymore.

And what should be the one we prefer?

I'd lean towards browser/locales/en-US/browser/linuxDesktopEntry.ftl instead. That's the source of truth we use for our official .deb package (bug 1824327 / D176588). I'd avoid using the one I added in bug 1390071 for 2 reasons:

1. Mozilla doesn't ship the Firefox Snap anymore, so this file is unused by actual users.
2. Back then, it was just a copy of what Canonical already had for Firefox. I didn't spend too much thinking of each entry like the ones you highlighted in comment 2.

If we want to improve the content of the .desktop file, I'd loop in :glandium who's the official maintainer of the Firefox package on Linux.

How does that sound to you, :gerard-majax?

Comment 7

[:gerard-majax]

(In reply to Johan Lorenzo [:jlorenzo] from comment #6)

What do you think we merge those differences together?

Let's do it! 👍 No need to maintain this duplication anymore.

And what should be the one we prefer?

I'd lean towards browser/locales/en-US/browser/linuxDesktopEntry.ftl instead. That's the source of truth we use for our official .deb package (bug 1824327 / D176588). I'd avoid using the one I added in bug 1390071 for 2 reasons:

Thanks for the FTL I was wondering how the localization was done since i saw hardcoded localization in the file

1. Mozilla doesn't ship the Firefox Snap anymore, so this file is unused by actual users.

Not, but official snap uses it: https://github.com/canonical/firefox-snap/blob/0a5f06d6e62f09a82f952afc48b3b167881dec60/snapcraft.yaml#L412-L414

Comment 8

[Martin Stránský [:stransky] (ni? me)]

(In reply to :gerard-majax from comment #3)

Martin, I suspect you might be the person to review that. What do you think we merge those differences together? And what should be the one we prefer?

There are a few entries where we might not want to merge: DBusActivatable, X-MultipleArgs for example. Also the Exec on Snap do not have %u (I believe because this partly broken due to Snap sandboxing, although an URL works fine here)

Looks okay to me, DBusActivatable should not be used unless you configure the system accordingly and ship proper DBus service file.

Comment 9

[:gerard-majax]

Attachment: Bug 1901703 - Change desktop_file generator to allow more customization r?gabriel!

Comment 10

[:gerard-majax]

Attachment: GitHub Pull Request

Comment 11

[:gerard-majax]

While Snap's build invokes already mach and we can easily integrate, I'm not sure how to deal with that for Flatpak; https://searchfox.org/mozilla-central/rev/6936c4c3fc9bee166912fce10104fbe0417d77d3/taskcluster/docker/firefox-flatpak/runme.sh#73

Comment 12

[:gerard-majax]

TIL thanks to comment 6 we also have another duplicate: https://searchfox.org/mozilla-central/rev/ab488b411ff15c2e11dbd4d6be0455bb64bc8fc1/python/mozbuild/mozbuild/repackaging/deb.py#517-651

I believe we need to get rid of all of those, and the best source of truth would be the deb one.

Comment 13

[:gerard-majax]

Martin, looking at the whole codebase I cannot see any direct use of https://searchfox.org/mozilla-central/source/browser/components/shell/search-provider-files/firefox.desktop, is it something that is being sourced by third party and which deleting from the tree would be problematic ?

Comment 14

[Martin Stránský [:stransky] (ni? me)]

(In reply to :gerard-majax from comment #13)

Martin, looking at the whole codebase I cannot see any direct use of https://searchfox.org/mozilla-central/source/browser/components/shell/search-provider-files/firefox.desktop, is it something that is being sourced by third party and which deleting from the tree would be problematic ?

Yes, we can remove https://searchfox.org/mozilla-central/source/browser/components/shell/search-provider-files/firefox.desktop

Comment 15

[:gerard-majax]

Attachment: Bug 1901703 - Remove duplicated desktop files r?stransky!

Comment 16

[:gerard-majax]

Attachment: Bug 1901703 - Extract desktop file generation from deb repackage r?gabriel!

Comment 17

[:gerard-majax]

could generate flatpak after much hacking:

$ ll ~/.local/share/flatpak/exports/share/applications/org.mozilla.firefox.desktop 
lrwxrwxrwx 1 alex alex 101 juil. 12 09:34 /home/alex/.local/share/flatpak/exports/share/applications/org.mozilla.firefox.desktop -> ../../../app/org.mozilla.firefox/current/active/export/share/applications/org.mozilla.firefox.desktop

Produced from https://firefox-ci-tc.services.mozilla.com/tasks/EKmOwz6lQ6-F53ghrvvmnw

Comment 18

[:gerard-majax]

Attachment: Bug 1901703 - Update 'repackage-deb' to changes for generated desktop file r?gabriel!

Comment 19

[:gerard-majax]

Attachment: Bug 1901703 - Update 'repackage-snap' to generate desktop file r?sergesanspaille!

Comment 20

[:gerard-majax]

Attachment: Bug 1901703 - Update 'release-flatpak-repackage' task for generated desktop file r?jlorenzo!

Comment 21

[Phabricator Automation]

Comment on attachment 9411799 [details]
Bug 1901703 - Remove duplicated desktop files r?stransky!

Revision D216050 was moved to bug 1907512. Setting attachment 9411799 [details] to obsolete.

Comment 22

[:gerard-majax]

Attachment: Bug 1901703 - Introduce mach repackage desktop-file command r?jlorenzo!

Comment 23

[:gerard-majax]

It's all good: https://treeherder.mozilla.org/jobs?repo=try&revision=ea394742758b33c52c2583db86bb126864f04280&selectedTaskRun=YppYRp6GTWKlFntyQMzFUw.0

Let's finish reviews and finalize making sure we have not regressed the actual content of the desktop file

Comment 26

[:gerard-majax]

Looks like there has been a lot of changes done in bug 1893603 that are potentially breaking this or at least not aligned

Comment 27

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/20a59c856abe
Extract desktop file generation from deb repackage r=gabriel
https://hg.mozilla.org/integration/autoland/rev/675e304c9f27
Change desktop_file generator to allow more customization r=gabriel
https://hg.mozilla.org/integration/autoland/rev/79248f3c40ea
Introduce mach repackage desktop-file command r=jlorenzo
https://hg.mozilla.org/integration/autoland/rev/750f19dd7a2e
Update 'repackage-deb' to changes for generated desktop file r=gabriel
https://hg.mozilla.org/integration/autoland/rev/52edc40e4c0d
Update 'repackage-snap' to generate desktop file r=sergesanspaille
https://hg.mozilla.org/integration/autoland/rev/52a358f2061a
Update 'release-flatpak-repackage' task for generated desktop file r=jlorenzo,releng-reviewers,jcristau,taskgraph-reviewers

Comment 28

[pstanciu]

https://hg.mozilla.org/mozilla-central/rev/20a59c856abe
https://hg.mozilla.org/mozilla-central/rev/675e304c9f27
https://hg.mozilla.org/mozilla-central/rev/79248f3c40ea
https://hg.mozilla.org/mozilla-central/rev/750f19dd7a2e
https://hg.mozilla.org/mozilla-central/rev/52edc40e4c0d
https://hg.mozilla.org/mozilla-central/rev/52a358f2061a

Comment 29

[Ryan VanderMeulen [:RyanVM]]

Is this really a regression from bug 1893603?

Comment 30

[:gerard-majax]

That depends on your definition of regression :). There were enough of risk that I prefered to mark as regressed because in a sense there was a regression.