As it stands, these two features are very similar and are in conflict. When both the design update and openViewOnFocus are enabled, both events occur. Furthermore, openViewOnFocus sends engagement telemetry and retained results does not. There is a test failure in browser_urlbar_event_telemetry.js when the Megabar is enabled because of this discrepancy; retained results overrides openViewOnFocus and no telemetry is sent in this subtest.

We should better distinguish these two features and what kind of telemetry they send. Marco proposed that openViewOnFocus occur when pageproxystate=valid or the search string is empty. Retained results could occur when pageproxystate=invalid and there is a non-empty search.

We should also figure out what kind of engagement telemetry retained results sends. Marco suggested a "returning" engagement or something along those lines.

This needs some brainstorming for sure.

Retained results

• acts when the input field is focused by the user or a tab switch or an activate event
• acts when the current string == the last query executed
• acts when we have cached results for the last query executed
• visual jank is reduced (reusing results)
• is enabled by default with the new design

openViewOnFocus

• acts when the user manually focuses the urlbar (mouse or keyboard)
• acts unless we are on an initial page (about:home, about:newtab)
• causes some visual jank (can't reuse results)
• it must be enabled through the openViewOnFocus pref

(In reply to Marco Bonardo [:mak] from comment #1)

openViewOnFocus

• acts when the user manually focuses the urlbar (mouse or keyboard)
• acts unless we are on an initial page (about:home, about:newtab)
• causes some visual jank (can't reuse ressults)

It's the same as clicking the dropmarker. Doesn't seem janky at all over here but I guess it depends on your profile and system.

• it must be enabled through the openViewOnFocus pref

Right, because we're still waiting for the results of the top sites experiment, I think? Honestly this process is very opaque and the timeline is unclear to me. Anyway, depending on what comes out of that we should either remove this mode or enable it by default. Either way the pref is expected to go away eventually.

I'm removing this from the megabar meta bug because at this point it seems the megabar should be ready for release before we'll know what to do with openViewOnFocus, and I don't think we want to block the megabar on this. (I also considered downgrading this to P3 and keep tracking it in the megabar meta bug; I don't care either way.)

(In reply to Dão Gottwald [::dao] from comment #2)

I'm removing this from the megabar meta bug because at this point it seems the megabar should be ready for release before we'll know what to do with openViewOnFocus, and I don't think we want to block the megabar on this. (I also considered downgrading this to P3 and keep tracking it in the megabar meta bug; I don't care either way.)

Re-reading comment 0, it seems that the talk about openViewOnFocus is somewhat of a distraction, and this bug is really about figuring out engagement telemetry for retained results? As in, we should have engagement telemetry for this regardless of whatever happens to openViewOnFocus? If so, it might make sense to have this block the megabar. Can you please clarify?

I think both are things to evaluate, we may want to keep this bug to check the overlapping of the 2 features, and one bug to check the engagement for retained results. So we must file the other bug too.

(In reply to Dão Gottwald [::dao] from comment #2)

It's the same as clicking the dropmarker. Doesn't seem janky at all over here but I guess it depends on your profile and system.

It should be, but it's not? it invokes startQuery without a searchString, that will use the current value. and it doesn't seem to check anywhere if the string is empty or pageproxystate is valid. Where do we limit it to work like the dropmarker?

To be clear, that is exactly the problem I was speaking about with Harry and the reason this bug was filed, the 2 features are strangely overlapping now, while they should not. openViewOnFocus was intended to be a replacement for the dropMarker, afaik.

(In reply to Marco Bonardo [:mak] from comment #4)

(In reply to Dão Gottwald [::dao] from comment #2)

It's the same as clicking the dropmarker. Doesn't seem janky at all over here but I guess it depends on your profile and system.

It should be, but it's not? it invokes startQuery without a searchString, that will use the current value. and it doesn't seem to check anywhere if the string is empty or pageproxystate is valid. Where do we limit it to work like the dropmarker?

startQuery uses an empty string for pageproxystate==valid: https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/browser/components/urlbar/UrlbarInput.jsm#819-820

(In reply to Dão Gottwald [::dao] from comment #6)

startQuery uses an empty string for pageproxystate==valid: https://searchfox.org/mozilla-central/rev/8bc24752246aeac8a9aed566cf1caccf88d97d11/browser/components/urlbar/UrlbarInput.jsm#819-820

yes, but what if it's not valid? I think openViewOnFocus should not act in such a case, while now there's nothing preventing it.

I've filed the telemetry bug so we can distinguish these two issues. I think the simplest solution here is to merge the two features: the UrlbarView is shown when the Urlbar is focused, except on the new tab page. Then there's no ambiguity.

That said, that would constitute a fairly significant design change, although it seems like the natural endpoint of replacing the dropmarker with openViewOnFocus. Plus Product and UX would have to get on board. Fixing the telemetry issue would resolve the issues here in the short term and we could bring this issue up Verdi so it's on his radar as he works on design update 2.

Bridget has asked that this not block the Top Sites meta so that meta can be closed. The megabar meta is tagged here too, so we're tracking it.

This bug has been coming up again in the planning and bug breakdown for 2020 Q1, being summarized as "Fix OpenViewOnFocus to act as a replacement for the dropmarker -- Currently it may also activate in other cases." I think there's a bit of a misunderstanding here. While this feature replaces the dropmarker, it was never supposed to exactly match it. At least that's not how I read any UX spec for this. So the mere fact that it activates in more cases doesn't necessarily present a problem to me.

Also making this block the megabar again since product management wants to release top sites together with the megabar.

It was designed before retained results, and the design was not re-evaluated after them. This makes me think the original intent may have been partially replaced by retained results. At a minimum we must confirm the behavior with Verdi.
Additionally, when it opens in other cases (non top sites), if results include search suggestions we still have "flicker" problems where suggestions will come sooner or later than the other results, and Verdi may not like that. While it's more acceptable to have that kind of "flicker" while the user is typing (results dynamically change while typing), here we just open a panel and focus user's attention on it.

(In reply to Marco Bonardo [:mak] from comment #11)

It was designed before retained results, and the design was not re-evaluated after them. This makes me think the original intent may have been partially replaced by retained results. At a minimum we must confirm the behavior with Verdi.

Agreed.

After some clarification with Verdi, here is what he wants:

1. we should always open the panel on focus
2. to avoid flicker in most cases, we should cache results for the last 5 searches (we can map searchstring => results)
3. these 2 features, were really imagined as one, as such they should be merged

This means imo:
a. move both features under the same pref, we can keep openViewOnFocus
b. merge some code paths, retained results can do pretty much the same as openViewOnFocus removing some bool checks, apart from the about:newtab/home checks
c. keep not opening on about:home or about:newtab, but open if there's a typed string (abandoned search) on them
d. once we move retained results under the same pref, if it's still disabled (top sites not there yet) it's ok to disable retained results too.

Please let me know of any problems not addressed here.

The Async Tab Switcher seems to fire the TabSelect event in parallel with focus
changes, making the behavior non-predictable, because we need a stable situation
where both events happened.

Depends on D60668

https://hg.mozilla.org/mozilla-central/rev/8ac85464e5ed
https://hg.mozilla.org/mozilla-central/rev/32f7019f6582