1198688 - Option to show search suggestions in location bar should be disabled in permanent private browsing mode

Status: VERIFIED FIXED

(Firefox :: Settings UI, defect, P3)

Description

[Petruta Horea [:phorea], Desktop QA]

Reproduced with Dev Edition 42.0a2 and Nightly 43.0a1 2015-08-25 across platforms.

Steps to reproduce:
1. Set the browser to permanent browse privately (never remember history)
2. Open about:preferences#search
3. Tick the "Show search suggestions in location bar results" checkbox 

Actual results:
The option can be selected

Expected results:
The second checkbox should be disabled in permanent private browsing mode (maybe in private windows too) as the search suggestions in location bar are not displayed when browsing privately.

Comment 1

[Marco Bonardo [:mak]]

ui glitch, not critical imo.
The problematic part is that permanent PB can be disabled elsewhere and in that case we might have to also uncheck the checkbox or it would look like disabled but active. So we will need to fake the pref status in the UI.

Comment 2

[Drew Willcoxon :adw]

Attachment: Screenshot of explanatory text when permanent PB mode is enabled

Stephen, I have a patch that disables the urlbar suggestions checkbox when permanent PB mode is enabled.  What do you think about showing some text explaining why it's disabled in that case, as this screenshot shows?

Comment 3

[Drew Willcoxon :adw]

Comment on attachment 8661394 [details]
Screenshot of explanatory text when permanent PB mode is enabled

Stephen said over IRC that he's fine with this.

Comment 4

[Drew Willcoxon :adw]

Attachment: patch

This (1) unchecks the urlbar suggestions checkbox whenever it's disabled, and (2) shows some explanatory text when permanent PB mode is enabled, as shown in the screenshot in comment 2.

Comment 5

[Dave Townsend [:mossop]]

Comment on attachment 8661409 [details] [diff] [review]
patch

Review of attachment 8661409 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/components/preferences/in-content/search.js
@@ +62,5 @@
>      this._initAutocomplete();
>  
> +    let suggestsPref =
> +      document.getElementById("browser.search.suggest.enabled");
> +    suggestsPref.addEventListener("change", () => {

Should we be listening for browser.privatebrowsing.autostart changes too?

Comment 6

[Drew Willcoxon :adw]

A previous version of this patch did, but I removed for simplicity's sake because the only way to toggle it in the UI is in the Privacy pane (I think?), and when you do that, you're forced to restart the browser.  Should I add it back?

Comment 7

[Dave Townsend [:mossop]]

(In reply to Drew Willcoxon :adw from comment #6)
> A previous version of this patch did, but I removed for simplicity's sake
> because the only way to toggle it in the UI is in the Privacy pane (I
> think?), and when you do that, you're forced to restart the browser.  Should
> I add it back?

Oh yeah forgot it needed a restart. No need then

Comment 8

[Drew Willcoxon :adw]

https://hg.mozilla.org/integration/fx-team/rev/bb4c100934bc

Comment 9

[Carsten Book [:Tomcat]]

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

Comment 10

[Petruta Horea [:phorea], Desktop QA]

Verified the message and checkbox in permanent private browsing mode using Dev Edition 43.0a2 2015-09-23 under Win 7 64-bit, Ubuntu 12.040 32-bit and Mac OS X 10.9.5.