1674150 - snap detection code disables profile management even when not packaged/run as a snap

Status: RESOLVED FIXED

(Core :: Widget: Gtk, defect, P3)

Description

[Olivier Tilloy]

User Agent: Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/86.0.4240.111 Safari/537.36

Steps to reproduce:

It's been reported several times on https://forum.snapcraft.io/ that when invoked from an application packaged as a classic snap¹ to open a hyperlink, firefox installed as a deb package (on Debian/Ubuntu) opens with the wrong profile by default.
This issue was investigated in https://launchpad.net/bugs/1835024, and we found out that the culprit is nsToolkitProfileService::IsSnapEnvironment()².

To reproduce the issue, ensure that a firefox window is open, then from a terminal run SNAP_NAME=foobar firefox http://example.org.

¹ https://snapcraft.io/docs/snap-confinement
² https://hg.mozilla.org/mozilla-central/file/tip/toolkit/profile/nsToolkitProfileService.cpp#l1838

Actual results:

This opens the requested URL (http://example.org) in a separate firefox window, with a different profile than that of the already open firefox window.

Expected results:

This should have opened the requested URL in a new tab in the same window that was already open.

Comment 1

[Olivier Tilloy]

Attachment: Bug 1674150 - Check for the actual snap name when detecting whether running as a snap.

Comment 2

[Virginia (please send need info request to Brindusa Tot)]

Hi Oliver,

Thank you for your report.

I was unable to reproduce this issue on Firefox Release version 81.0 (64-bit) Mozilla Firefox for Ubuntu canonical 1.0 , but I did reproduce the issue by downloading Firefox Nightly 84.0a1 (2020-11-14) (64-bit) and installing it on a different location (for example in Desktop Folder "Nightly").

I'll add this ticket to the Widget GTK component in the hope their team can take a look at this and share their feedback.

Regards,
Virginia

Comment 3

[Olivier Tilloy]

(In reply to Virginia Balducci from comment #2)

I'll add this ticket to the Widget GTK component in the hope their team can take a look at this and share their feedback.

Thanks for triaging Virginia. Unfortunately the patch¹ I submitted almost a month ago hasn't received any feedback yet. How do I get someone to look at it?

¹ https://phabricator.services.mozilla.com/D95171

Comment 4

[Johan Lorenzo [:jlorenzo]]

Olivier reached out to me on Element. I looked at the patch and it seems the owner of nsToolkitProfileService.cpp is :mossop these days[1]. Would you have the bandwidth to review this patch, Dave?

[1] https://hg.mozilla.org/mozilla-central/log/93fabad45659d172b723c9606215d3acaab54df1/toolkit/profile/nsToolkitProfileService.cpp

Comment 5

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

I'm Gtk peer so you can ask me to review linux/gtk code.

Comment 6

[Dave Townsend [:mossop]]

I vaguely recall some plan to distribute devedition as a separate snap at some point which this would break but it looks like that hasn't happened so this is fine. I really wish there was a sane API to detect this though, environment variable detection always seemed likely to be flakey.

Comment 8

[Dorel Luca [:dluca]]

Hi,
Phabricator show this bug has this tag : testing-exception-other (Please leave a comment explaining why).

Removing check-in needed because of that.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/51d1c7feaa42
Check for the actual snap name when detecting whether running as a snap. r=stransky

Comment 10

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/51d1c7feaa42

Comment 11

[Sophie Winter]

This is back. SNAP_NAME=foo firefox (when Firefox is not installed as a snap) opens up the wrong profile in FF 104, and it appears the check for a specific snap name has been removed.

Comment 12

[Dave Townsend [:mossop]]

(In reply to Sophie Winter from comment #11)

This is back. SNAP_NAME=foo firefox (when Firefox is not installed as a snap) opens up the wrong profile in FF 104, and it appears the check for a specific snap name has been removed.

It would be best to file a new bug for this. It will have been regressed by bug 1297520

Comment 13

[Mike Kaply [:mkaply]]

1. Why are you setting SNAP_NAME?
2. That made it so we don't check SNAP_NAME at all (only SNAP_INSTANCE_NAME) so I don't think it should have affected this (unless I'm reading my own code wrong :) )

Comment 14

[Dave Townsend [:mossop]]

(In reply to Mike Kaply [:mkaply] from comment #13)

1. Why are you setting SNAP_NAME?
2. That made it so we don't check SNAP_NAME at all (only SNAP_INSTANCE_NAME) so I don't think it should have affected this (unless I'm reading my own code wrong :) )

The different environment name is one thing. But because we no longer check that the name is "correct" any snap app calling a non-snap Firefox will now cause Firefox to think it is running as a snap again.

Comment 15

[Mike Kaply [:mkaply]]

The different environment name is one thing. But because we no longer check that the name is "correct" any snap app calling a non-snap Firefox will now cause Firefox to think it is running as a snap again.

Doh. Definitely file a bug. I'll update the patch.

Comment 16

[Dave Townsend [:mossop]]

(In reply to Mike Kaply [:mkaply] from comment #15)

The different environment name is one thing. But because we no longer check that the name is "correct" any snap app calling a non-snap Firefox will now cause Firefox to think it is running as a snap again.

Doh. Definitely file a bug. I'll update the patch.

For reference this is bug 1791442