1445660 - Policy: Set whether or not search bar is displayed

Status: VERIFIED FIXED

(Firefox :: Enterprise Policies, defect)

Description

[Mike Kaply [:mkaply]]

This came up in discussion with search team.

We should provide a policy to allow the search bar to be displayed by default (versus using the unified toolbar).

I do this in CCK2, so it should be straightforward.

Comment 1

[Mike Kaply [:mkaply]]

Attachment: Bug 1445660 - Add policy for enabling the search bar.

Review commit: https://reviewboard.mozilla.org/r/232236/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/232236/

Comment 2

[Mike Kaply [:mkaply]]

felipe:

Looking for feedback on naming, etc.

I remember you sending an email with recommendations, but I can't find it.

Comment 3

[:Felipe Gomes (needinfo for replies!)]

Ah, the recommendation was in bug 1449318, which I believe is a dupe of this?

The suggestion was:

policy name: SearchBar
type: string
enum: ["unified", "separate"]

Comment 5

[Mike Kaply [:mkaply]]

Clearly I have a short memory opening up the same bug within two weeks.

I'll  update the patch.

Comment 6

[:Felipe Gomes (needinfo for replies!)]

Fwiw, I saw the e-mail discussion about not using the pref, but I'm still confused. The pref "browser.sarch.widget.inNavBar" is exactly what about:preferences use: https://searchfox.org/mozilla-central/source/browser/components/preferences/in-content/search.xul#13

Two questions:

- Does CustomizableUI automatically re-syncs that pref to display the information correctly in about:preferences? and,
- When that pref is dropped, will this be the right method going forward to change the search bar setting?

If any of the answers is no, we should probably just use the pref, and let whoever handles the pref removal to do the right conversion here too.

If the answer is yes to both, then we can go ahead. I think we should get Paolo's or Drew's review here to make sure this is ok.

Comment 7

[Mike Kaply [:mkaply]]

> Does CustomizableUI automatically re-syncs that pref to display the information correctly in about:preferences?

Yep.

> When that pref is dropped, will this be the right method going forward to change the search bar setting?

Yes. This is the recommended mechanism per the team. We've been told not to use the pref.

Drew: Can you confirm you're OK with this approach?

Comment 8

[Drew Willcoxon :adw]

Looks good to me, jibes with what was discussed in the email thread.

Comment 9

[Mike Kaply [:mkaply]]

Comment on attachment 8963340 [details]
Bug 1445660 - Add policy for enabling the search bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232236/diff/1-2/

Comment 10

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8963340 [details]
Bug 1445660 - Add policy for enabling the search bar.

https://reviewboard.mozilla.org/r/232236/#review238076

Looks good, but your test will leave the browser in a different state. Since the default is now unified, I think that at the end of the test you should setupPolicyEngineWithJSON again with SearchBar: unified.

Comment 11

[Mike Kaply [:mkaply]]

Comment on attachment 8963340 [details]
Bug 1445660 - Add policy for enabling the search bar.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/232236/diff/2-3/

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/c0da25f5a462
Add policy for enabling the search bar. r=Felipe

Comment 13

[Narcis Beleuzu [:NarcisB]]

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

Comment 14

[:Felipe Gomes (needinfo for replies!)]

[Tracking Requested - why for this release]:
Enterprise Policies

Comment 15

[:Felipe Gomes (needinfo for replies!)]

Comment on attachment 8963340 [details]
Bug 1445660 - Add policy for enabling the search bar.

Approval Request Comment
[Feature/Bug causing the regression]: Enterprise Policies
[User impact if declined]: Choose whether the search bar appears as unified
[Is this code covered by automated tests?]: yes
[Has the fix been verified in Nightly?]: not yet
[Needs manual test from QE? If yes, steps to reproduce]: qa will handle it
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: no
[Why is the change risky/not risky?]: contained to this policy being in use
[String changes made/needed]: none

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8963340 [details]
Bug 1445660 - Add policy for enabling the search bar.

Policy Engine fix needed for ESR60. Approved for 60.0b9.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/5dc3e48f679d

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0702a754328d
Follow-up, make SearchBar policy enterprise-only. r=me

Comment 19

[:Felipe Gomes (needinfo for replies!)]

When I uplifted this patch (comment 17), I took the liberty to add `"enterprise_only": true` to this policy, following the principle that any policy related to search should be enterprise-only.

So I landed this now on inbound too to keep things in sync:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0702a754328d4e220a67c53161c2be15401de373

Comment 20

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/0702a754328d

Comment 21

[Abe - QA (:Abe_LV)]

We tested this on Nightly with JSON policy format and it is verified as fixed.

Using this policy, the search bar can be displayed or unified by default.

It will also be tested with adm formats when ready.
Test cases and runs are here- https://testrail.stage.mozaws.net/index.php?/plans/view/8760

Comment 22

[Abe - QA (:Abe_LV)]

We retested this on beta builds[FX60] with ADM and JSON policy formats and it is verified as fixed.

Test cases and runs are here- https://testrail.stage.mozaws.net/index.php?/plans/view/8760