Closed
Bug 512305
Opened 15 years ago
Closed 6 years ago
Set Firefox as the default web browser using xdg-settings
Categories
(Firefox :: Shell Integration, enhancement)
Tracking
()
RESOLVED
FIXED
Firefox 60
People
(Reporter: thestig, Assigned: olivier)
References
Details
Attachments
(2 files, 2 obsolete files)
59 bytes,
text/x-review-board-request
|
jhorak
:
review+
RyanVM
:
approval-mozilla-release+
|
Details |
2.52 KB,
patch
|
Details | Diff | Splinter Review |
User-Agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.13) Gecko/2009080317 Ubuntu/8.04 (hardy) Firefox/3.0.13 Build Identifier: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9.0.13) Gecko/2009080317 Ubuntu/8.04 (hardy) Firefox/3.0.13 The Chromium web browser uses the xdg-settings tool [1] on Linux to figure out what the default web browser is, and set itself as the default web browser if the user requested it. It would be great if all Linux web browsers have a common way of getting/setting the default web browser. Can we change Firefox to use xdg-settings if it's available? Xdg-settings is not widely deployed yet, but it should be included in the next xdg-utils release. Reproducible: Always
Comment 1•13 years ago
|
||
I'll second this. The feature has never worked for me as implemented and unless I disable checking, Firefox always asks whether I want it to make itself the default browser, even after it's purportedly already done so.
Comment 2•6 years ago
|
||
Snap packages (powered by Ubuntu) would benefit from this feature. Olivier Tilloy (cc'd) reported we can't set Firefox as the default browser (when packaged as Snap) without it. Olivier pointed out the code currently lives at [1]. It uses gconf. He added we could do something similar to what Chromium does: > xdg-settings set default-web-browser firefox.desktop Finally, he said xdg-settings inside a snap doesn't work yet, but it is currently being implemented in snapd. [1] https://dxr.mozilla.org/mozilla-central/rev/c38d22170d6f27e94c3c53093215d20255fab27a/browser/components/shell/nsGNOMEShellService.cpp#250
Blocks: snappy
Assignee | ||
Comment 4•6 years ago
|
||
Here is a first shot at using xdg-settings to get/set the default browser. Feedback welcome.
Comment 5•6 years ago
|
||
Comment on attachment 8947234 [details] [diff] [review] use-xdg-settings.patch Martin, you've reviewed other code in this file. Could you take a look?
Attachment #8947234 -
Flags: review?(stransky)
Comment 6•6 years ago
|
||
Comment on attachment 8947234 [details] [diff] [review] use-xdg-settings.patch Jan usually does this system integration stuff so moving to him. I guess the xdg can be also used for Flatpak builds.
Attachment #8947234 -
Flags: review?(stransky) → review?(jhorak)
Comment 7•6 years ago
|
||
Comment on attachment 8947234 [details] [diff] [review] use-xdg-settings.patch Review of attachment 8947234 [details] [diff] [review]: ----------------------------------------------------------------- ::: browser/components/shell/nsGNOMEShellService.cpp @@ +129,5 @@ > +{ > + if (PR_GetEnv("SNAP") != nullptr) { > + return "firefox_firefox.desktop"; > + } > + return "firefox.desktop"; What current implementation allows is to download upstream binary and set it as a default browser because the GIO creates and install the .desktop for us (by giovfs->CreateAppFromCommand). This won't be possible with your changes. You can try to use xdg-settings as a fallback implementation if the gio one fails rather than replacing it.
Attachment #8947234 -
Flags: review?(jhorak) → review-
Comment 8•6 years ago
|
||
Olivier: Is this something you can do? We're trying to get this change in fairly soon.
Flags: needinfo?(olivier)
Assignee | ||
Comment 9•6 years ago
|
||
Jan: thanks for the review! Mike: I should be able to get to it within the next couple of days.
Flags: needinfo?(olivier)
Comment 10•6 years ago
|
||
Olivier: Is this by chance something you'll be able to do soon? We were hoping to get it in Firefox 59 (although that's very tight at this point) Thanks
Assignee | ||
Comment 11•6 years ago
|
||
Attaching an updated patch that preserves the existing logic and uses xdg-settings only when running as a snap.
Attachment #8954711 -
Flags: review?(jhorak)
Comment 12•6 years ago
|
||
Is PR_GetEnv("SNAP") the best way to check for a Snap?
Comment 13•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #12) > Is PR_GetEnv("SNAP") the best way to check for a Snap? It's the simplest way, especially if we don't want to add any further dependencies.
Assignee | ||
Comment 14•6 years ago
|
||
(In reply to Mike Kaply [:mkaply] from comment #12) > Is PR_GetEnv("SNAP") the best way to check for a Snap? We could be a tad more specific and compare the value of $SNAP_NAME to "firefox". But that's essentially the same mechanism. Note that this makes it convenient to test the functionality when running a local build, not packaged as a snap: export SNAP before running and you simulate the snap confinement.
Comment 15•6 years ago
|
||
Sorry, I should have marked this earlier. [Tracking Requested - why for this release]: We are looking to release a Firefox Snap for 59. This is a Snap only fix for 59. I plan to have it in today.
tracking-firefox59:
--- → ?
Comment 16•6 years ago
|
||
So look at this further (and Jan's comments), this seems like a temporary Snap solution as opposed to a full xdg-settings solution. I'll try to get it in for now, but ideally we should do this:
> You can try to use xdg-settings as a fallback implementation if the gio one fails rather than replacing it.
Comment hidden (mozreview-request) |
Comment 18•6 years ago
|
||
mozreview-review |
Comment on attachment 8954868 [details] Bug 512305 - Use xdg-settings for Firefox Snap. https://reviewboard.mozilla.org/r/224014/#review230174
Attachment #8954868 -
Flags: review?(jhorak) → review+
Comment 19•6 years ago
|
||
See: https://treeherder.mozilla.org/#/jobs?repo=try&revision=b774fd54f6f3f8be6434527017428af0bccdd038&selectedJob=165200234 /builds/worker/workspace/build/src/browser/components/shell/nsGNOMEShellService.cpp:218:58: error: ISO C++ forbids converting a string constant to 'gchar* {aka char*}
Comment 20•6 years ago
|
||
Please address the issues in try run (see above).
Comment 21•6 years ago
|
||
mozreview-review |
Comment on attachment 8954868 [details] Bug 512305 - Use xdg-settings for Firefox Snap. https://reviewboard.mozilla.org/r/224014/#review230298
Attachment #8954868 -
Flags: review+
Assignee | ||
Comment 22•6 years ago
|
||
Updated patch that fixes the build error (it was only a warning when I built it locally, I should have been more careful).
Attachment #8947234 -
Attachment is obsolete: true
Attachment #8954711 -
Attachment is obsolete: true
Attachment #8954711 -
Flags: review?(jhorak)
Attachment #8955182 -
Flags: review?(jhorak)
Comment hidden (mozreview-request) |
Comment 24•6 years ago
|
||
Assuming this would need to land for 60 as well.
Comment 25•6 years ago
|
||
Once this lands on mozilla-central can you make sure someone requests uplift to m-r?
Flags: needinfo?(mozilla)
Comment 26•6 years ago
|
||
mozreview-review |
Comment on attachment 8954868 [details] Bug 512305 - Use xdg-settings for Firefox Snap. https://reviewboard.mozilla.org/r/224014/#review230552 Good. Thanks.
Attachment #8954868 -
Flags: review?(jhorak) → review+
Comment 27•6 years ago
|
||
Comment on attachment 8954868 [details] Bug 512305 - Use xdg-settings for Firefox Snap. Approval Request Comment [Feature/Bug causing the regression]: Setting default browser on Snap isn't working [User impact if declined]: No ability to set default browser on Snap [Is this code covered by automated tests?]: No [Has the fix been verified in Nightly?]: No [Needs manual test from QE? If yes, steps to reproduce]: No [List of other uplifts needed for the feature/fix]: None [Is the change risky?]: No [Why is the change risky/not risky?]: Snap/Linux specific. [String changes made/needed]: None
Flags: needinfo?(mozilla)
Attachment #8954868 -
Flags: approval-mozilla-release?
Comment 28•6 years ago
|
||
Pushed by mozilla@kaply.com: https://hg.mozilla.org/integration/autoland/rev/581dc5ffd572 Use xdg-settings for Firefox Snap. r=jhorak
Comment 29•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/581dc5ffd572
Status: UNCONFIRMED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 60
Updated•6 years ago
|
Assignee: nobody → olivier
Comment 30•6 years ago
|
||
Comment on attachment 8954868 [details] Bug 512305 - Use xdg-settings for Firefox Snap. Fixes setting Firefox as the default browser when using Snap builds. Approved for 59rc1.
Attachment #8954868 -
Flags: approval-mozilla-release? → approval-mozilla-release+
Comment 31•6 years ago
|
||
bugherder uplift |
https://hg.mozilla.org/releases/mozilla-beta/rev/6cefff30c2a4 (FIREFOX_59b_RELBRANCH) https://hg.mozilla.org/releases/mozilla-release/rev/74e95f9fb43e
Updated•6 years ago
|
Attachment #8955182 -
Flags: review?(jhorak)
You need to log in
before you can comment on or make changes to this bug.
Description
•