Misleading message when DisableAppUpdate is true but the system administrator updates Firefox in place

RESOLVED FIXED in Firefox 67

Status

()

defect
P5
normal
RESOLVED FIXED
8 months ago
3 months ago

People

(Reporter: ambrose.li, Assigned: phoenixgyaan, Mentored)

Tracking

({good-first-bug})

64 Branch
Firefox 67
Points:
---
Bug Flags:
qe-verify +

Firefox Tracking Flags

(firefox67 fixed)

Details

Attachments

(2 attachments)

Reporter

Description

8 months ago
User Agent: Mozilla/5.0 (X11; Linux x86_64; rv:64.0) Gecko/20100101 Firefox/64.0

Steps to reproduce:

1. Install Nightly as root so that it’s not writable by any non-root user (and in particular, Firefox cannot update itself)

2. Create policy file with contents “{"policies": { "DisableAppUpdate": true }}”

4. Update Firefox (as system administrator) so that binaries in the original installation path are changed


Actual results:

3. Edit|Preferences confirms that “Your organization has disabled the ability to change some preferences.” and “Updates disabled by your system administrator.”

5. At some point, opening a new tab produces the error message “We have just installed an update in the background. Click Restart Nightly to finish applying it.” and Firefox refuses to open any page.



Expected results:

Since Firefox did not update itself, it is misleading to say “We have just installed an update in the background”. When DisableAppUpdate is true, the message should be changed to “Your system administrator has just installed an update in the background” (mirroring wording in Edit|Preferences).
Mozilla/5.0 (X11; Linux x86_64; rv:65.0) Gecko/20100101 Firefox/65.0 (20181101100640)

I've tried to test this report on Ubuntu 16.04 using the latest Nightly and Beta builds. However, I have trouble installing or starting the browser as root even if I`m an administrator of the user. I got the following error: 

root@us16-04x64vm:/home/svuser/Downloads/Beta/firefox# ./firefox -p
Running Firefox as root in a regular user's session is not supported.  ($XAUTHORITY is /home/svuser/.Xauthority which is owned by svuser.)

How did you install Firefox as a root? Can you provide any hint?

Moving this to Firefox: Enterprise Policies.
Component: Untriaged → Enterprise Policies
Flags: needinfo?(ambrose.li)
Reporter

Comment 2

8 months ago
To install Nightly as root I download and unpack the tar.bz2/tar.gz file (I have a script to do this and on one of my machines it’s run nightly via cron). After installing you run it as a normal user, not as root.
Flags: needinfo?(ambrose.li)
I was able to install the browser as root and to set the mentioned policy. When navigating to about:preferences under Nightly Update, check for update is greyed out. Opening a new tab does not produce the error message “We have just installed an update in the background. Click Restart Nightly to finish applying it.”

Could you provide a picture or a short video showing the problem? 
Do you see any errors in about:policies?
Flags: needinfo?(ambrose.li)
Reporter

Comment 4

8 months ago
There’s no errors in about:policies, and you wouldn’t see the behaviour if you just installed it and opened a new tab. You have to install it, keep the browser running, wait for a day (i.e., a new Nightly), then install the new Nightly over your existing Nightly. Then, if you’re lucky, Firefox will detect that its binaries have changed and display the warning message.

The problem is that the message is misleading. It suggests that Firefox was able to update itself in the background even when theoretically that’s impossible. So when the user is also the system administrator the wording makes it look like Firefox has either a bug or a backdoor of some kind, when what happened was the system administrator has updated Firefox. The message should reflect that reality and it currently isn’t.
Flags: needinfo?(ambrose.li)
Reporter

Comment 5

8 months ago
wait a day (i.e., wait for a new nightly)*
Making that page detect what caused the update (if it was Firefox or the sysadmin who changed the files) will be hard. However, I think a simple re-wording of that phrase will make it more neutral and solve the confusion that is being reported here. How about we change it:

From: We have just installed an update in the background.
To: Firefox has just been updated in the background.


Stephen, what do you think about this change? Should we check with someone else about making this change? (i.e., was there an UX or copywriter person who wrote the original string?)
Mentor: felipc
Status: UNCONFIRMED → NEW
Ever confirmed: true
Flags: needinfo?(spohl.mozilla.bugs)
Keywords: good-first-bug
Priority: -- → P5
(In reply to :Felipe Gomes (needinfo me!) from comment #6)
> Making that page detect what caused the update (if it was Firefox or the
> sysadmin who changed the files) will be hard. However, I think a simple
> re-wording of that phrase will make it more neutral and solve the confusion
> that is being reported here. How about we change it:
> 
> From: We have just installed an update in the background.
> To: Firefox has just been updated in the background.
> 
> 
> Stephen, what do you think about this change? Should we check with someone
> else about making this change? (i.e., was there an UX or copywriter person
> who wrote the original string?)

This was recommended by UX in bug 1366808. Forwarding your question to Bram.
Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(bram)

(In reply to :Felipe Gomes (needinfo me!) from comment #6)

From: We have just installed an update in the background.
To: Firefox has just been updated in the background.

Hi Felipe, this is similar to the copy that Stephen initially proposed. I’m okay with changing it from “we have” to “Firefox has”.

Flags: needinfo?(bram)

Comment 9

4 months ago

I would like to work on this.
I am currently building Firefox, please let me know if this can be assigned to me!
Thanks.

Assignee

Comment 10

4 months ago

Hi, I am Phoenix, a participant for the current Outreachy round.
I would like to work on this, do we need a complete build for this, or will an artefact build suffice?

We will take a patch from anyone and whoever can post a patch will get this bug assigned to. The line to change is:

https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/aboutRestartRequired.ftl#7

Instructions on how to submit a patch can be found here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

Assignee

Comment 12

4 months ago

(In reply to Stephen A Pohl [:spohl] from comment #11)

We will take a patch from anyone and whoever can post a patch will get this bug assigned to. The line to change is:

https://searchfox.org/mozilla-central/source/browser/locales/en-US/browser/aboutRestartRequired.ftl#7

Instructions on how to submit a patch can be found here:

https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/How_to_Submit_a_Patch

I am making a patch right now, with [spohl] as reviewer

Assignee

Comment 13

4 months ago

Changed text
From: We have just installed an update in the background.
To: Firefox has just been updated in the background.

Assignee: nobody → phoenixgyaan
Status: NEW → ASSIGNED

Comment 14

4 months ago
Pushed by fgomes@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/04fb2d9beeac
Misleading message when DisableAppUpdate is true but the system administrator updates Firefox in place r=spohl,flod

Comment 15

4 months ago
bugherder
Status: ASSIGNED → RESOLVED
Closed: 4 months ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 67

Comment 16

4 months ago

(In reply to Oana Pop-Rus from comment #15)

https://hg.mozilla.org/mozilla-central/rev/04fb2d9beeac

-restart-required-intro = We have just installed an update in the background. Click Restart { -brand-short-name } to finish
+restart-required-intro-brand = { -brand-short-name } has just been updated in the background. Click Restart { -brand-short-name } to finish
applying it.

"Applying it" made sense in the old string, but doesn’t seem to do so when "an update" is left out in the new one. How about "Click Restart { -brand-short-name } to finish the update."?

(In reply to Ton [:Tonnes] from comment #16)

(In reply to Oana Pop-Rus from comment #15)

https://hg.mozilla.org/mozilla-central/rev/04fb2d9beeac

-restart-required-intro = We have just installed an update in the background. Click Restart { -brand-short-name } to finish
+restart-required-intro-brand = { -brand-short-name } has just been updated in the background. Click Restart { -brand-short-name } to finish
applying it.

"Applying it" made sense in the old string, but doesn’t seem to do so when "an update" is left out in the new one. How about "Click Restart { -brand-short-name } to finish the update."?

Flags: needinfo?(bram)

Comment 18

4 months ago

Correction for the suggestion: How about "Click Restart { -brand-short-name } to finish applying the update." (or "~ to complete the update", or better)?

Assignee

Comment 19

4 months ago

Once it's decided as to what message should be shown, I'll create a patch and update this.

Flags: needinfo?(spohl.mozilla.bugs)

(In reply to Stephen A Pohl [:spohl] from comment #17)

How about "Click Restart { -brand-short-name } to finish the update."?

(In reply to Ton [:Tonnes] from comment #18)

How about "Click Restart { -brand-short-name } to finish applying the update." (or "~ to complete the update", or better)?

I’d like to suggest something that would clarify the fact that the update has been downloaded, but still needs to be installed.

The first part of the message can go like this:

Alternative 1:
A { -brand-short-name } update is available.

Alternative 2:

A { -brand-short-name } update has been downloaded.

Alternative 3:

A {-brand-short-name} update has been downloaded in the background.

The second part of the sentence can be worded like this:

Alternative A:
Restart to install this update now.

Alternative B:
Restart now to apply the update.

Alternative C:
Restart to update { -brand-short-name}.

I would suggest combining 1 + A or 1 + B. If we want even more clarify into the background process, use 2 or 3.

Thoughts?

Flags: needinfo?(bram)

(In reply to Bram Pitoyo [:bram] from comment #20)

Thoughts?

This isn't technically correct because not only has the update been downloaded, it has actually already been installed/applied by the time this message is displayed. This message is displayed when the actual binaries on disk have changed and an old, currently running instance of Firefox is attempting to launch a new content process. Since the binary for content processes has changed, the new process could crash as it is not compatible with the old, currently running version of Firefox.

I do like both suggestions made in comment 18.

Flags: needinfo?(spohl.mozilla.bugs) → needinfo?(bram)

(In reply to Stephen A Pohl [:spohl] from comment #21)

This isn't technically correct because not only has the update been downloaded, it has actually already been installed/applied by the time this message is displayed.

I also like comment #18. Let’s go with the second message, then:

"~ to complete the update"

Flags: needinfo?(bram)
Flags: needinfo?(phoenixgyaan)
Assignee

Comment 23

4 months ago

I'll make a patch with the message as:
"Click Restart { -brand-short-name } to complete the update."

Flags: needinfo?(phoenixgyaan)

Comment 25

4 months ago
Pushed by spohl@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/ac2880eb346a
Misleading message when DisableAppUpdate is true but the system administrator updates Firefox in place. r=spohl,flod
Flags: qe-verify+
QA Contact: emil.ghitta
You need to log in before you can comment on or make changes to this bug.