I see events where selected_result is unknown, but results contain autofill_origin or search_engine.
There is a string typed.
engagement_type is enter.

I found a possible case where this happens, if the user types a string and then moves with the keyboard to a search shortcut, and confirms with SHIFT+ENTER (skip search mode) then we record unknown as selected_result, we should probably annotate the user has picked a shortcut button

Another case is typing a search, pressing ESC to close the results panel, then confirm the search with Enter, should return input_field as selected_result, instead it returns unknown.
Also why is selected_result not empty if there's no selected result? unknown should only be used when there is a selected result but we can't recognize it, maybe there's a catch-all setting to unknown even if there isn't a selected result.

Thank you very much!

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

I found a possible case where this happens, if the user types a string and then moves with the keyboard to a search shortcut, and confirms with SHIFT+ENTER (skip search mode) then we record unknown as selected_result, we should probably annotate the user has picked a shortcut button

Yes, I also could reproduce this. In the above case, as no element is selected, such is the record.
The easiest way to fix it might be that selects the first row (I think this behavior is the same as selecting the first row) if view.visibleResults is not empty, at below.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarSearchOneOffs.sys.mjs#213
But, selecting a row changes the UI (very short time though), so it might not be right way. Or, we may be able to just use the first row as the selected result, before taking the telemetry.

And also, one thing. How about the following STR?

1. Set 0 to browser.urlbar.maxRichResults.
2. Seach with @google mozilla.

The result of the telemetry now is:

search_mode:"", 
n_results:0, 
selected_result: "input_field",
provider: null,

In this case, actually, we show a temporary result created below.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarInput.sys.mjs#713-723
I think we better take the above result, what do you think?

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

Another case is typing a search, pressing ESC to close the results panel, then confirm the search with Enter, should return input_field as selected_result, instead it returns unknown.

I also could reproduce this as well.
It seems that the reason is that even if we close the results by ESC, the view.visibleResults still remain. So, we may need to check view.isOpen first here.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarController.sys.mjs#1096

Also why is selected_result not empty if there's no selected result? unknown should only be used when there is a selected result but we can't recognize it, maybe there's a catch-all setting to unknown even if there isn't a selected result.

This happens since the results still remain, I think.

(In reply to Daisuke Akatsuka (:daisuke) from comment #3)

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

Another case is typing a search, pressing ESC to close the results panel, then confirm the search with Enter, should return input_field as selected_result, instead it returns unknown.

I also could reproduce this as well.
It seems that the reason is that even if we close the results by ESC, the view.visibleResults still remain. So, we may need to check view.isOpen first here.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarController.sys.mjs#1096

Ah, no, it is not so simple. Since the results will be closed before here in most cases.
Perhaps, we may need to reset visibleResults when closing by ESC.

(In reply to Daisuke Akatsuka (:daisuke) from comment #4)

(In reply to Daisuke Akatsuka (:daisuke) from comment #3)

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

Another case is typing a search, pressing ESC to close the results panel, then confirm the search with Enter, should return input_field as selected_result, instead it returns unknown.

I also could reproduce this as well.
It seems that the reason is that even if we close the results by ESC, the view.visibleResults still remain. So, we may need to check view.isOpen first here.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarController.sys.mjs#1096

Ah, no, it is not so simple. Since the results will be closed before here in most cases.
Perhaps, we may need to reset visibleResults when closing by ESC.

Or, we may better clear it when Enter and if the view is closed.

clearing stuff in the context object is not a good idea, we risk to break other consumers.
It'd be better to detect the case when we collect telemetry.

Yes, thanks.

How about this case?

(In reply to Daisuke Akatsuka (:daisuke) from comment #3)

And also, one thing. How about the following STR?

1. Set 0 to browser.urlbar.maxRichResults.
2. Seach with @google mozilla.

The result of the telemetry now is:

search_mode:"", 
n_results:0, 
selected_result: "input_field",
provider: null,

In this case, actually, we show a temporary result created below.
https://searchfox.org/mozilla-central/rev/cd2121e7d83af1b421c95e8c923db70e692dab5f/browser/components/urlbar/UrlbarInput.sys.mjs#713-723
I think we better take the above result, what do you think?

Should we add search_mode at least?

Depends on D172098

(In reply to Daisuke Akatsuka (:daisuke) from comment #3)

And also, one thing. How about the following STR?

1. Set 0 to browser.urlbar.maxRichResults.
2. Seach with @google mozilla.

The result of the telemetry now is:

search_mode:"", 
n_results:0, 
selected_result: "input_field",
provider: null,

For correctness we can set search_mode yes, though that is a low priority, because to have meaningful analysis I'd suggest to exclude users who set maxRichResults = 0. That's a really edge case. Of course if the fix is simple it won't hurt.

Comment on attachment 9322044 [details]
Bug 1817208: Check wheather the view is opening when recording

Revision D172098 was moved to bug 1822210. Setting attachment 9322044 [details] to obsolete.

https://hg.mozilla.org/mozilla-central/rev/0f49451cff74

I've managed to reproduce the issue using an Fx 112.0a1 from 2023-03-06.
The issue is verified fixed using the latest Fx 113.0a1 on Windows 10 and Ubuntu 22.04. The selected_result for the engagement telemetry ping is now "search_shortcut_button" when the user performs the search using shift+enter with a search shortcut selected.

uplift to 112, just in case we want to measure there

Comment on attachment 9322293 [details]
Bug 1817208: Introduce search_shortcut_button type for immediate search on oneoff button

Beta/Release Uplift Approval Request

• User impact if declined: This change is for telemetry. So, no user impact.
  But, we want to know how the user uses the urlbar and the results more correctly to improve the user experience.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): This affects only telemetry.
• String changes made/needed: None
• Is Android affected?: Unknown

Comment on attachment 9322293 [details]
Bug 1817208: Introduce search_shortcut_button type for immediate search on oneoff button

Approved for 112.0b4

https://hg.mozilla.org/releases/mozilla-beta/rev/a4dbbf8b9473

The issue is verified fixed using the latest Fx 112.0b5 on Windows 10 and Ubuntu 22. The selected_result for the engagement telemetry ping is now "search_shortcut_button" when the user performs the search using shift+enter with a search shortcut selected.