1828282 - Import bookmarks button on Bookmarks Toolbar bypasses DisableProfileImport enterprise policy

Status: RESOLVED FIXED

(Firefox :: Enterprise Policies, defect, P3)

Description

[aoia7rz7l]

+++ This bug was initially created as a clone of Bug #1828280 +++

When I was investigating bug 1828280 I noticed that the Import bookmarks button on Bookmarks Toolbar also managed to bypass the DisableProfileImport enterprise policy, even though it was supposedly fixed in bug 1676439. However, this only seems to happen when I was using an older profile (e.g. one created in ESR102) on the latest Nightly (114.0a1), but not when using a vanilla Nightly profile.

Prerequisites:

1. Set the DisableProfileImport policy to true.
2. Use an older profile (e.g. ESR102) that is not created in the latest Nightly.

STR:

Set browser.toolbars.bookmarks.visibility to either newtab or always if the Bookmarks Toolbar does not show up.

Expected Behavior:

The Import Bookmarks button does not appear in the Bookmarks Toolbar when the DisableProfileImport policy is set to true (see bug 1676439).

Actual Behavior:

The Import Bookmarks button appeared in the Bookmarks Toolbar and is clickable, even when the DisableProfileImport policy is set to true.

mozgression --good 78 --bad 84 (because it's still bad in 102 nightly) returned

Last good revision: 5b5ffbe4add97c98003026e0d9a730302161b646
First bad revision: 7d6d66062e843a75b7aafb4ec0ae2dff355755e7
Pushlog : https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5b5ffbe4add97c98003026e0d9a730302161b646&tochange=7d6d66062e843a75b7aafb4ec0ae2dff355755e7
Switching bisection method to taskcluster
...
There are no build artifacts for these changesets (they are probably too old).

so my guess would be bug 1665442, although that would imply that it was broken from the beginning?

Comment 1

[BugBot [:suhaib / :marco]]

The Bugbug bot thinks this bug should belong to the 'Firefox::Bookmarks & History' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 2

[:Gijs (he/him)]

The change in bug 1676439 prevented this button from being added to the toolbar. If you're using a toolbar where the item has already been added, I guess that's why the fix "doesn't work". But this also means that users can bypass the policy by adding the import button themselves (so either way there's a "gap" here).

Can you actually reproduce this if you create the profile in ESR102, given the change from 1676439 predates that ESR?

ISTM that there should (also) be a change somewhere "lower down" in the import code that causes it to no-op, rather than only whack-a-mole-ing all the potential entrypoints for import, if avoiding import is really this important. (I must confess I don't really understand why enterprise would want to prevent import, other than that I'd expect any policy that prevents e.g. adding bookmarks to also prevent import. But if users can recreate the data themselves, preventing them from importing it seems pointless, other than making people's lives harder. Ni Mike for this.)

Comment 3

[Mike Kaply [:mkaply]]

It's probably worth preventing it deeper down as necessary if we are going to keep adding entry points.

As far as why enterprises disable it, I can't answer that but it was a requested feature by enterprises.

Comment 4

[aoia7rz7l]

(In reply to :Gijs (he/him) from comment #2)

Can you actually reproduce this if you create the profile in ESR102, given the change from 1676439 predates that ESR?

Yes. I actually first noticed this on my Nightly profile, but then wasn't able to reproduce it in mozregression so I ran mozregression --good 102 --bad 113 --profile /path/to/ESR102_profile, but the supposedly good builds were bad, so I tried mozregression --good 91 --bad 102 --profile /path/to/ESR102_profile (and so on), until I ended up with the result in comment 0. Tabs did start to consistently crash on startup towards the end, although I am not sure if that's relevant here.

Comment 5

[Mike Kaply [:mkaply]]

I double checked and it works with a new profile (as designed). It was never intended to remove the button from an existing profile.

Comment 6

[:Gijs (he/him)]

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

I double checked and it works with a new profile (as designed). It was never intended to remove the button from an existing profile.

Out of interest, why not? If the feature should be disabled, shouldn't the button not be there in any profile?

Comment 7

[Mike Kaply [:mkaply]]

Out of interest, why not? If the feature should be disabled, shouldn't the button not be there in any profile?

Probably, but at the time, it was easiest to just not add the button. I'll take this bug for implementing a better solution.

Comment 8

[:Gijs (he/him)]

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

Out of interest, why not? If the feature should be disabled, shouldn't the button not be there in any profile?

Probably, but at the time, it was easiest to just not add the button. I'll take this bug for implementing a better solution.

Did you mean to assign this bug to yourself? :-)

Comment 9

[Mike Kaply [:mkaply]]

Did you mean to assign this bug to yourself? :-)

Yep, I'm going to fix it.

Comment 10

[Mike Kaply [:mkaply]]

Attachment: Bug 1828282 - Remove import button if policy is added. r?Gijs

Comment 11

[BugBot [:suhaib / :marco]]

The severity field is not set for this bug.
:mkaply, could you have a look please?

For more information, please visit BugBot documentation.

Comment 13

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/8532d2e24a07
https://hg.mozilla.org/integration/autoland/rev/72e933a42128
Remove import button if policy is added. r=Gijs,firefox-desktop-core-reviewers ,places-reviewers,Standard8

Comment 14

[amarc]

https://hg.mozilla.org/mozilla-central/rev/72e933a42128