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

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, defect, P5)

Description

[Ambrose Li]

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).

Comment 1

[Stefan [:StefanG_QA]]

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.

Comment 2

[Ambrose Li]

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.

Comment 3

[Stefan [:StefanG_QA]]

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?

Comment 4

[Ambrose Li]

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.

Comment 5

[Ambrose Li]

wait a day (i.e., wait for a new nightly)*

Comment 6

[:Felipe Gomes (needinfo for replies!)]

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?)

Comment 7

[Stephen A Pohl [:spohl]]

(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.

Comment 8

[Bram Pitoyo [: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”.

Comment 9

[Neha]

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

Comment 10

[PhoenixAbhishek]

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?

Comment 11

[Stephen A Pohl [:spohl]]

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

Comment 12

[PhoenixAbhishek]

(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

Comment 13

[PhoenixAbhishek]

Attachment: Bug 1501543 - Misleading message when DisableAppUpdate is true but the system administrator updates Firefox in place

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

Comment 14

[Pulsebot]

Pushed by [e-mail]:
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

[Oana Pop-Rus]

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

Comment 16

[Ton [:Tonnes]]

(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."?

Comment 17

[Stephen A Pohl [:spohl]]

(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."?

Comment 18

[Ton [:Tonnes]]

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

Comment 19

[PhoenixAbhishek]

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

Comment 20

[Bram Pitoyo [:bram]]

(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?

Comment 21

[Stephen A Pohl [:spohl]]

(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.

Comment 22

[Bram Pitoyo [: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"

Comment 23

[PhoenixAbhishek]

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

Comment 24

[PhoenixAbhishek]

Attachment: Bug 1501543 - Misleading message when DisableAppUpdate is true but the system administrator updates Firefox in place. r=spohl

Updated the message strings

Comment 25

[Pulsebot]

Pushed by [e-mail]:
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

Comment 26

[Stefan Hindli [:stefan_hindli]]

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