Closed Bug 1367427 Opened 3 years ago Closed 3 years ago

The one-offs bar is not displayed in the Awesome Bar while search suggestions hint is displayed

Categories

(Firefox :: Address Bar, defect, P1)

55 Branch
defect

Tracking

()

VERIFIED FIXED
Firefox 55
Tracking Status
firefox-esr52 --- unaffected
firefox53 --- unaffected
firefox54 --- unaffected
firefox55 --- verified

People

(Reporter: simona.marcu, Assigned: mak)

References

Details

(Keywords: regression, Whiteboard: [fxsearch])

Attachments

(3 files)

Attached image missingOneoffs.png
Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:55.0) Gecko/20100101 Firefox/55.0
Build ID: 20170523125628

[Affected versions]:
- Nightly 55.0a1

[Affected platforms]:
- All

[Steps to reproduce]:
1. Launch the latest Nightly with a new profile
2. Type something in the Awesome Bar

[Expected result]:
- The One-Offs bar should be displayed at the bottom of the Awesome's bar drop-down

[Actual result]:
- The One-offs bar is not displayed at the bottom of the Awesome's bar drop-down


[Additional notes]:
- This issue is not reproducible on the Nightly 55.0a1 from 2017-03-23 (Build ID: 20170523030206)
- The One-offs bar is not displayed as long as the Search Suggestions hint is displayed (this apply to all the Firefox sessions where the search suggestions hint is displayed).
- The issue is always reproducible on clean profiles and on the profiles where the search suggestions were ignored (Yes/No was not selected in the search suggestions urlbar opt-in notification)
Priority: -- → P1
Whiteboard: [fxsearch]
Status: NEW → ASSIGNED
Regression range:
Last good revision: 2fde169875b3358a319695cb3007d452b9d5f920
First bad revision: 830c06ded9bb474004bbf8aaf9d8f9b44d811147
Pushlog:
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=2fde169875b3358a319695cb3007d452b9d5f920&tochange=830c06ded9bb474004bbf8aaf9d8f9b44d811147
it's a pre-existing bug uncovered by the fact now we show the onboarding notification on focus. We hide the footer element, and we never unhide it. I just need some time to check tests, the fix is pretty much ready.
Component: Search → Location Bar
Blocks: 1192503
I found an additional problem due to some recent changes to the browser code, now _urlbarFocused seems to be "broken" for our use-case.

Mike Conley changed its behavior in bug 1362866.
Our comment seems to tell if _urlbarFocused is set, the urlbar popup would be closed by tabbrowser, that made sense for oldBrowser, but now when we create newBrowser we set its _urlbarFocused as well.

Additionally bug 1192503 already hinted that we should remove this... Now we do show the hint on focus, but we need a different way to figure out if a browser may be going away.

For now I will just drop this "optimization", since its complication seems to overweight its usefulness... but would be great if there would be another way to tell that a browser is about to be switched from.
Mike, any idea if there's another way I could tell that a browser is being switched-off?
Flags: needinfo?(mconley)
note, it's also possible that after bug 1362866 we are less likely to wrongly focus a browser that is being switched off.
Additionally, now opening a new tab causes a "focus" event only if the locationbar wasn't focused in the previous tab, thus further opening new tabs won't re-focus the urlbar.
We show the onboarding on a new tab only if the previous one didn't have focus on the urlbar. I guess we'll have to adapt for performance reasons, opening a new tab is an important operation.

To sum up:
* we show the onboarding when the urlbar gets focus and before something else was focused
  - that means we don't show onboarding if a new tab is opened when the urlbar already had focus
* we show the onboarding if the user starts typing
* globally we show it 4 times, but a mouse focus MAY show it a 5th time per design requirements if all the previous 4 times were activated by the keyboard.
Panos, feel free to forward the review to Drew if you don't feel comfortable at looking to this.
Keywords: regression
Attachment #8870880 - Flags: review?(past) → review?(adw)
I'm out of time today.
Comment on attachment 8870880 [details]
Bug 1367427 - The one-offs bar is not displayed in the Awesome Bar while search suggestions hint is displayed.

https://reviewboard.mozilla.org/r/142452/#review146046

LGTM, glad we can get rid of the _urlbarFocused checks, and the test changes look solid.
Attachment #8870880 - Flags: review?(adw) → review+
Pushed by mak77@bonardo.net:
https://hg.mozilla.org/integration/autoland/rev/7a9918ba3731
The one-offs bar is not displayed in the Awesome Bar while search suggestions hint is displayed. r=adw
https://hg.mozilla.org/mozilla-central/rev/7a9918ba3731
Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: in-testsuite+
Resolution: --- → FIXED
Target Milestone: --- → Firefox 55
Verified as fixed using the latest Nightly 55.0a1 (Build ID: 20170525030225) on Windows 10 x64, Ubuntu 16.04 x64 and Mac OS X 10.12.
Status: RESOLVED → VERIFIED
(In reply to Marco Bonardo [::mak] from comment #3)
> Mike, any idea if there's another way I could tell that a browser is being
> switched-off?

Is the TabSelect event too late?
Flags: needinfo?(mconley) → needinfo?(mak77)
(In reply to Mike Conley (:mconley) from comment #13)
> (In reply to Marco Bonardo [::mak] from comment #3)
> > Mike, any idea if there's another way I could tell that a browser is being
> > switched-off?
> 
> Is the TabSelect event too late?

The problem we were handling was that we were getting a request to focus the locationbar, but immediately that browser was switched to another one, so the request was pointless. Now we get less focus requests so potentially we don't need that anymore.

We could use TabSelect to show the onboarding but I think it's not worth the added code.
Flags: needinfo?(mak77)
> status-firefox53: --- → unaffected

Is that certain? I have symptoms of the bug with 53.0.2 on FreeBSD-CURRENT.
Firefox 53 doesn't have the feature enabled, it's enabled in 55.
Thanks. 

Found: 

browser.urlbar.oneOffSearches

I had used it for … probably many months, never realising that it was not a standard feature. I guess that somehow, the user setting was lost. It's now 'true' and working in 54.0.
You need to log in before you can comment on or make changes to this bug.