Search suggestions opt-in in the urlbar should not appear in private windows

VERIFIED FIXED in Firefox 42

Status

()

defect
P1
normal
Rank:
7
VERIFIED FIXED
4 years ago
4 years ago

People

(Reporter: adw, Assigned: adw)

Tracking

Trunk
Firefox 43
Points:
---
Dependency tree / graph
Bug Flags:
firefox-backlog +

Firefox Tracking Flags

(firefox41 unaffected, firefox42 verified, firefox43 verified)

Details

(Whiteboard: [suggestions][fxsearch])

Attachments

(2 attachments)

Assignee

Description

4 years ago
Search suggestions in the urlbar are disabled in private windows, so the opt-in notification should not be shown there.
Flags: firefox-backlog+
Assignee

Comment 1

4 years ago
Posted patch patchSplinter Review
Marco, not sure how available you are to do reviews right now, but I'll give it a shot. :-)
Attachment #8649563 - Flags: review?(mak77)
Comment on attachment 8649563 [details] [diff] [review]
patch

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

::: browser/base/content/test/general/browser_urlbarSearchSuggestionsNotification.js
@@ +139,5 @@
> +  gURLBar.blur();
> +  gURLBar.focus();
> +  yield promiseAutocompleteResultPopup("foo");
> +  assertVisible(true);
> +  gURLBar.blur();

is this whole check really needed? I assume the other tests are already checking that the notification is visible in a normal window...
Attachment #8649563 - Flags: review?(mak77) → review+

Updated

4 years ago
Rank: 7
Assignee

Comment 4

4 years ago
Yeah, fair enough, I landed without it.

https://hg.mozilla.org/integration/fx-team/rev/eba63213016d
https://hg.mozilla.org/mozilla-central/rev/eba63213016d
Status: ASSIGNED → RESOLVED
Last Resolved: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 43
Assignee

Comment 6

4 years ago
Approval Request Comment
[Feature/regressing bug #]:
Search suggestions opt-in notification in the urlbar

[User impact if declined]:
The opt-in will incorrectly and confusingly appear in private windows.

[Describe test coverage new/current, TreeHerder]:
New automated test

[Risks and why]:
Very low risk, minor one-line change with automated and manual test coverage

[String/UUID change made/needed]:
None
Attachment #8650720 - Flags: approval-mozilla-aurora?
Comment on attachment 8650720 [details] [diff] [review]
Aurora/42 patch (same as patch that landed on fx-team/m-c)

Thanks for the tests! Taking it.
Attachment #8650720 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed using Developer Edition 42.0a2 and Nightly 43.0a1 both 2015-08-23 under Win 7 64-bit, Ubuntu 14.04 and Mac OS X 10.9.5.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.