1668389 - Outline of address bar focus ring obscured when in search mode with no engines or results

Status: RESOLVED FIXED

(Firefox :: Address Bar, defect, P2)

Description

[Cameron McCormack (:heycam)]

Attachment: screen shot

See the attached screen shot. When I press Ctrl+K, the orange (here on Linux) focus ring around the address bar is not painted on the bottom edge, which looks a bit weird.

Comment 1

[Harry Twyford]

I can't reproduce on macOS. Cameron, it seems like you don't have any one-offs or search history in your screenshot, which is unexpected. Have you unchecked most/all of the engines under about:preferences#search > Search Shortcuts? Also what is the value of the pref browser.urlbar.maxHistoricalSearchSuggestions?

Comment 2

[Cameron McCormack (:heycam)]

Yes, that is exactly what I've done. I have removed all but DuckDuckGo from the Search Shortcuts list. I also have unchecked "Provide search suggestions". My browser.urlbar.maxHistoricalSearchSuggestions value is 2, which looks like the default.

Comment 3

[Harry Twyford]

Ah, now I can reproduce. Thank you.

Comment 4

[Harry Twyford]

Hm, I can no longer reproduce this, including with a fresh profile and by rolling back to a Nightly from a few days ago. Cameron, can you still reproduce on the latest Nightly? If so, could you please post the values of browser.urlbar.update2 and all the browser.urlbar.update2.* prefs?

Comment 5

[Cameron McCormack (:heycam)]

I can still reproduce on current Nightly.

browser.urlbar.update2 true
browser.urlbar.update2.disableOneOffsHorizontalKeyNavigation true
browser.urlbar.update2.emptySearchBehavior 2
browser.urlbar.update2.localOneOffs true
browser.urlbar.update2.oneOffsRefresh true
browser.urlbar.update2.restyleBrowsingHistoryAsSearch true
browser.urlbar.update2.tabToComplete true

Comment 6

[Marco Bonardo [:mak]]

This is what is overlapping
https://searchfox.org/mozilla-central/rev/4352fb7b0d17c1febff9569ed311e0e42c93093e/browser/themes/shared/urlbarView.inc.css#64

Comment 7

[Harry Twyford]

Thanks, Marco.

Comment 8

[Harry Twyford]

Attachment: Bug 1668389 - Only set #urlbar[open] if we are sure there will be either results or one-offs. r?adw,mak

This was more difficult to solve than I expected. The main issue is that the
[open] attribute on #urlbar wasn't accurate when the view was "open" but
there were no results or one-offs, so it was in fact closed. This broke a few
style rules.

This bug can also be reached when the user has no engines and clears the search
string while in search mode. This includes pressing Accel+K when typing a search
string while not in search mode.

The relationship between the UrlbarView and the one-offs is complex and depends
on a lot of listeners and async calls made in synchronous contexts. Furthermore,
most of the UrlbarView open/close code is synchronous, but checking the number
of engines (to determine if the one-offs will open) is an async operation. These
factors make it difficult for the view to discern any state about the one-offs
and plan accordingly.

First I tried adding a [oneoff] attribute to .urlbarView, set asynchronously when
the one-offs are built. Then I changed CSS rules to check

.urlbarView:not([noresults]),
.urlbarView[oneoffs]

instead of just #urlbar[open]. This approach would've required changing
a bunch of CSS from the simple #urlbar[open] to the more complicated CSS
above, which was not good for code readability. Also it would keep [open] in
a weird useless state where it couldn't really be trusted. This would've caused
other styling problems.

I settled on adding a .then call around the affected UrlbarView opening. The
view opens only if we are sure that we will have either results or one-offs,
so we can once again trust the [open] parameter. This approach has its
drawbacks. Mainly, it's a more JavaScript-heavy solution so it's possibly
slow. Thankfully, it's something that can be easily cached.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5be393d34bbd
Only set #urlbar[open] if we are sure there will be either results or one-offs. r=mak

Comment 10

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/5be393d34bbd