1438180 - "Show search suggestions in address bar results" checkbox from the about:preferences#search page has a misleading behavior

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P1)

Description

[Emil Ghitta, QA [:emilghitta]]

Attachment: issue.gif

[Affected versions]:
60.0a1 (BuildId:20180213220104)
59.0b9 (BuildId:20180209162511)
58.0.2 (BuildId:20180206200532)

[Unaffected versions]:
52.6.0 esr (BuildId:20180118122319)

[Affected platforms]:
Windows 10 64bit.
macOS 10.13.
Ubuntu 16.04 64bit.

[Steps to reproduce]:
1. Launch Firefox with a clean profile.
2. Access the about:preferences#search page.
3. Untick the "Provide search suggestions" checkbox.
4. Restart Firefox.
5. Access the about:preferences#search page.

[Expected result]:
The following checkboxes are displayed garyed out and unticked:
- "Show search suggestions in address bar results".
- "Show search suggestions ahead of browsing history in address bar results".

[Actual result]:
The "Show search suggestions in address bar results" checkbox is displayed ticked and grayed out.


[Regression range]:
This is a regression:

Last good revision: 5c50e8581fedf2ef3a218574f705c71cc20e2cd8
First bad revision: 62517cf482ae592f7d71afb6d2819c7d7801615c

Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=5c50e8581fedf2ef3a218574f705c71cc20e2cd8&tochange=62517cf482ae592f7d71afb6d2819c7d7801615c

[Note]

Please note that the "Show search suggestions ahead of browsing history in address bar results" checkbox is displayed under the "Default Search Engine" only in Nightly for the moment (per bug 1432716).

For further information regarding this issue please observe the attached screencast.

Comment 1

[Emil Ghitta, QA [:emilghitta]]

Hi Mark!

It seems that mozregression pointed out Bug 1393437.

Can you please have a look into this?

Thanks a lot!

Comment 2

[Mark Banner (:standard8)]

A faster way to reproduce this is to just close preferences & re-open it, although that doesn't always reproduce in latest nightly from the looks of it.

I've double-checked and the regression does seem to be bug 1393437.

However, I'm confused as I'm only adding a new preferences field, and not modifying anything else.

It seems to be the state of "Show search suggestions in address bar results" after restart is reflecting its saved value.

Since I don't normally do much preferences, I'm a bit confused. Need-info to Jared/Gijs to see if they have any ideas.

Comment 3

[:Gijs (he/him)]

Attachment: Bug 1438180 - don't update the url bar search suggestions pref directly,

In search.js, _updateSuggestionCheckboxes is already hooked up to update this checkbox
based on the preference value, including change handlers for when it changes. As a result,
having both the preference attribute and the 'manual' JS means that the actual checked and
disabled state races with which code sets the value first.

The search suggestion order preference already doesn't have a preference attribute and is
only ever updated via the JS. It seems sensible to do the same here.

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

Comment 4

[Myk Melez [:myk] [@mykmelez]]

Comment on attachment 8951692 [details]
Bug 1438180 - don't update the url bar search suggestions pref directly,

I'm not a Firefox peer, so I can't grant r+, but this fix seems reasonable to me.

Comment 6

[Marco Bonardo [:mak] (Away Apr 25 - May 5)]

this will be worth an uplift.

Comment 7

[:Gijs (he/him)]

[Tracking Requested - why for this release]:
I think we need to avoid shipping this if we can.

Comment 8

[Jared Wein [:jaws] (please needinfo? me)]

Comment on attachment 8951692 [details]
Bug 1438180 - don't update the url bar search suggestions pref directly,

https://reviewboard.mozilla.org/r/220976/#review227920

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/ba1d6f897a92
don't update the url bar search suggestions pref directly, r=jaws,f=myk

Comment 10

[Natalia Csoregi [:nataliaCs]]

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

Comment 11

[Ryan VanderMeulen [:RyanVM]]

Please nominate this for Beta approval when you get a chance. Also, let's get QE to verify the fix on Nightly.

Comment 12

[Emil Ghitta, QA [:emilghitta]]

This issue is verified fixed using Firefox 60.0a1 (BuildId:20180221220150) on Windows 10 64bit, macOS 10.13.3 and Ubuntu 16.04 64bit. 

Leaving the qe-verify+ flag until this issue is verified in beta as well.

Comment 13

[:Gijs (he/him)]

Comment on attachment 8951692 [details]
Bug 1438180 - don't update the url bar search suggestions pref directly,

Approval Request Comment
[Feature/Bug causing the regression]: race condition that is apparently triggered by bug 1393437
[User impact if declined]: prefs don't behave the way they should
[Is this code covered by automated tests?]: apparently not in a way that catches this
[Has the fix been verified in Nightly?]: not yet.
[Needs manual test from QE? If yes, steps to reproduce]: see comment 0 and comment 2
[List of other uplifts needed for the feature/fix]: n/a
[Is the change risky?]: no
[Why is the change risky/not risky?]: single-line change that's been looked at by 3 people by now, will only affect this preference and, tbh, at this point it'd be hard to make the behaviour of this UI pref a lot worse...
[String changes made/needed]: no

Comment 14

[:Gijs (he/him)]

(In reply to :Gijs from comment #13)
> [Has the fix been verified in Nightly?]: not yet.

Err, sorry, I hadn't seen comment 12 - yes, it's been verified! :-)

Comment 15

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8951692 [details]
Bug 1438180 - don't update the url bar search suggestions pref directly,

Fix for a confusing UI issue, verified on Nightly already. Taking for 59b12.

Comment 16

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/releases/mozilla-beta/rev/df143d7bcd19

Comment 17

[Timea Zsoldos [:zstimi/tzsoldos], Desktop QA]

I have reproduced this issue using Firefox  60.0a1 (2018.02.14) on Win 8.1 x64.
I can confirm this issue is fixed, I verified using Firefox 59.0b12 on Windows 8.1 x64, Ubuntu 14.04 x64 and Mac OS X 10.13.4.

Comment 18

[Liz Henry (:lizzard) (relman/hg->git project)]

Retroactive tracking in case this reopens. Which I doubt will happen, but hey.

Comment 19

[:Gijs (he/him)]

Backed out from mozilla-release for causing bug 1443229.


https://hg.mozilla.org/releases/mozilla-release/rev/c98eb35cfd6d181f11211f191c04c14efb548e49