Closed Bug 1194669 Opened 5 years ago Closed 5 years ago
Prompt to opt-in/out search suggestions still shown after answered in another window
Reproduced with Nightly 43.0a1 20150813030208 under all platforms. Steps to reproduce: 1. Open several browser windows 2. Start typing in the urlbar - notice the prompt to opt-in/out search suggestions 3. Select Yes OR NO 4. Select another window and type in the urlbar AR: The prompt is displayed again. ER: The prompt shouldn't be displayed again if the user already made his choice Note: Couldn't check on Dev Edition 42.0a2 (2015-08-13) where setting the prefs browser.urlbar.userMadeSearchSuggestionsChoice;true and browser.urlbar.suggest.searches;true doesn't display the prompt
Priority: -- → P1
Reproduced with Aurora 42.0a2 with browser.urlbar.unifiedcomplete set to true.
OK, this is only a problem for the windows where you've typed in the urlbar before making a choice. If you just open a bunch of windows, go to one, start typing in the urlbar, and then make a choice, then the other windows correctly do not show the notification when you type in them.
Instead of hiding the notification directly when Yes or No is clicked, have the pref observer hide it. Then the urlbar where Yes or No is clicked only sets the pref, and the pref observers of all the urlbars hide their notifications.
Comment on attachment 8650810 [details] [diff] [review] patch Review of attachment 8650810 [details] [diff] [review]: ----------------------------------------------------------------- The new `yield transitionPromise`s in the test were necessary to prevent failures where, despite also yielding on the searchPromise, the test continued before the search suggestions actually appeared. Not sure why.
Comment on attachment 8650810 [details] [diff] [review] patch Clearing review, try saw the same failures that I thought I had fixed locally.
There were a couple of problems: (1) The new multipleWindows test should wait for the notification transition to finish, not for the search to complete, before checking whether the notification is visible. (2) For some reason the condition that promiseSearchComplete waits for seems to be wrong in this case: return win.gURLBar.controller.searchStatus >= Ci.nsIAutoCompleteController.STATUS_COMPLETE_NO_MATCH; So instead of waiting for that condition, wait for suggestions to actually appear in the results. And the suggestions are actually appearing when the test fails, it's just that they're appearing after promiseSearchComplete's condition happens. It's not a meaningful failure. (3) This might not actually be a problem, but I made the test wait for a pref observer to fire every time it sets the userMadeSearchSuggestionsChoice pref, since that's what causes the notification to be hidden. I made this change before the previous two fixes, and I'm not sure it's necessary anymore, but it can't hurt.
Most recent try looks good.
Comment on attachment 8651284 [details] [diff] [review] patch v2 Approval Request Comment [Feature/regressing bug #]: Search suggestions opt-in notification in the urlbar, bug 959567 [User impact if declined]: If you have multiple windows open and dismiss the notification in one window, it will still appear in the other windows. [Describe test coverage new/current, TreeHerder]: New automated test, manual testing [Risks and why]: Low risk: well tested, still plenty of time to bake on Aurora before Beta merge, and this patch only affects the notification (but the patch touches urlbar code to do that) [String/UUID change made/needed]: None
Attachment #8651284 - Flags: approval-mozilla-aurora?
Comment on attachment 8651284 [details] [diff] [review] patch v2 OK, let's take it!
Attachment #8651284 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Verified as fixed using Aurora 42.0a2 and Nightly 43.0a1 2015-09-01.
You need to log in before you can comment on or make changes to this bug.