Closed
Bug 1399968
Opened 7 years ago
Closed 7 years ago
browser.search.widget.inNavBar isn't set correctly on startup
Categories
(Firefox :: Settings UI, defect, P1)
Firefox
Settings UI
Tracking
()
RESOLVED
FIXED
Firefox 57
Tracking | Status | |
---|---|---|
firefox57 | --- | fixed |
People
(Reporter: standard8, Assigned: standard8)
References
Details
(Whiteboard: [fxsearch])
Attachments
(1 file)
Bug 1387416 changed the browser.search.widget.inNavBar to default to false, and it also attempted to change it to be set correctly on start up according to its state in CustomizableUI. However, looking into bug 1393437 comment 24, I've found that the pref is always set to false on startup, regardless of if the search box is in toolbar or not.
Updated•7 years ago
|
Status: NEW → ASSIGNED
Comment hidden (mozreview-request) |
Comment 2•7 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #0) > Bug 1387416 changed the browser.search.widget.inNavBar to default to false, > and it also attempted to change it to be set correctly on start up according > to its state in CustomizableUI. > > However, looking into bug 1393437 comment 24, I've found that the pref is > always set to false on startup, regardless of if the search box is in > toolbar or not. I'm confused, bug 1393437 comment 24 looks like it would happen if the pref was set to true, not false... Can you clarify how this patch addresses things? Do we somehow init the tracker before we've loaded user prefs or something? I don't really understand why the prefs would mismatch...
Flags: needinfo?(standard8)
Assignee | ||
Comment 3•7 years ago
|
||
(In reply to :Gijs from comment #2) > (In reply to Mark Banner (:standard8) from comment #0) > > Bug 1387416 changed the browser.search.widget.inNavBar to default to false, > > and it also attempted to change it to be set correctly on start up according > > to its state in CustomizableUI. > > > > However, looking into bug 1393437 comment 24, I've found that the pref is > > always set to false on startup, regardless of if the search box is in > > toolbar or not. > > I'm confused, bug 1393437 comment 24 looks like it would happen if the pref > was set to true, not false... Can you clarify how this patch addresses > things? Do we somehow init the tracker before we've loaded user prefs or > something? I don't really understand why the prefs would mismatch... At startup SearchWidgetTracker.init() is called before the toolbar elements have been initialised, so when it calls SearchWidgetTracker.syncPreferenceWithWidget() -> widgetIsInNavBar() then 'placement' comes back as null, so the preference is always set to false on startup. Later on, toolbar.xml kicks in and initialises the toolbar, and then widgetIsInNavBar will return the right value. Hence the move to waiting for the onAreaNodeRegistered event. Bug 1393437 comment 24 has the search box in the toolbar, so the pref value should be "true", but due to the above startup sequence, it actually gets set to "false". The preferences display correctly displays the incorrectly set value.
Flags: needinfo?(standard8)
Comment 4•7 years ago
|
||
mozreview-review |
Comment on attachment 8908316 [details] Bug 1399968 - Ensure the inNavBar search pref really does get set correctly on startup. https://reviewboard.mozilla.org/r/179944/#review185396
Attachment #8908316 -
Flags: review?(gijskruitbosch+bugs) → review+
Comment 5•7 years ago
|
||
(In reply to Mark Banner (:standard8) from comment #3) > At startup SearchWidgetTracker.init() is called before the toolbar elements > have been initialised, so when it calls > SearchWidgetTracker.syncPreferenceWithWidget() -> widgetIsInNavBar() then > 'placement' comes back as null, so the preference is always set to false on > startup. > > Later on, toolbar.xml kicks in and initialises the toolbar, and then > widgetIsInNavBar will return the right value. Hence the move to waiting for > the onAreaNodeRegistered event. > > Bug 1393437 comment 24 has the search box in the toolbar, so the pref value > should be "true", but due to the above startup sequence, it actually gets > set to "false". The preferences display correctly displays the incorrectly > set value. This makes sense, thanks for the explanations!
Pushed by mbanner@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/956bc408307c Ensure the inNavBar search pref really does get set correctly on startup. r=Gijs
Comment 7•7 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/956bc408307c
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I think the bug is not fixed yet. I use the "general.config.filename" to provide users with a centralized configuration. Here I have defined the value: pref("browser.search.widget.inNavBar", true); --> However, this value is reset to false at startup. I also tested: lockPref("browser.search.widget.inNavBar", true); --> The value remains correctly in the configuration, but it is not effective. The search window is not displayed. Test environment: Firefox Quantum 58.0 (64-Bit)
Comment 9•6 years ago
|
||
(In reply to Susi from comment #8) > I think the bug is not fixed yet. I use the "general.config.filename" to > provide users with a centralized configuration. Here I have defined the > value: pref("browser.search.widget.inNavBar", true); --> However, this value > is reset to false at startup. > > I also tested: lockPref("browser.search.widget.inNavBar", true); --> The > value remains correctly in the configuration, but it is not effective. The > search window is not displayed. > > Test environment: Firefox Quantum 58.0 (64-Bit) I don't think locking the pref or generally using autoconfig with it is a supported usecase. Paolo, can you clarify?
Flags: needinfo?(paolo.mozmail)
Comment 11•6 years ago
|
||
Thank you for your prompt reply! Do I understand the article correctly, that the "Policy Engine" is only supported from Firefox 60 ESR? Not with the current 58.0.x - non ESR version?
Comment 12•6 years ago
|
||
(In reply to Susi from comment #11) > the "Policy Engine" is only supported from Firefox 60 ESR? Yes, and it's still in development (Firefox Nightly is currently version 60). The comment in bug 1425199 says that when working on the policy engine we *could* look into having a setting for this. It's up to the team working on the policy engine whether they add such a setting. Felipe, is this on your radar/roadmap? > Not with the current 58.0.x - non ESR version? Correct.
Flags: needinfo?(felipc)
Comment 13•6 years ago
|
||
(In reply to :Gijs from comment #12) > (In reply to Susi from comment #11) > > the "Policy Engine" is only supported from Firefox 60 ESR? > > Yes, and it's still in development (Firefox Nightly is currently version > 60). The comment in bug 1425199 says that when working on the policy engine > we *could* look into having a setting for this. It's up to the team working > on the policy engine whether they add such a setting. Felipe, is this on > your radar/roadmap? This request hadn't come up to us before, so it's not on our list of policies to support in 60. But it is a reasonable request. I suggest filing a bug in Firefox :: Enterprise Policies. I don't think we can work on this on the timeline for 60, but I'd happily accept a patch for it.
Flags: needinfo?(felipc)
You need to log in
before you can comment on or make changes to this bug.
Description
•