You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.
At the end of the day this code searches the Google SearchEngine instance only, with the check above. Should we simplify this?
If we want to keep it generic for supporting more search engines in the future then:
Should we check resultUrls instead of suggestUrl since the latter one is the URL we query search suggestions from and doesn't have to be related to the search engine URL. The result URLs may be closer.
Should we also search in *.state.search.availableEngines? Otherwise you will not find a search engine if the user decided to hide it.
In appendSearchAttributionToUrlIfNeeded() we already change the URL to the one with the search attribution, so we already know the context, it seems complicated to then check this here again. 🤔 This would also allow us to directly lookup the search engine (e.g. Google) and not have to search for it here, I assume?
At the end of the day this code searches the Google
SearchEngine
instance only, with the check above. Should we simplify this?If we want to keep it generic for supporting more search engines in the future then:
resultUrls
instead ofsuggestUrl
since the latter one is the URL we query search suggestions from and doesn't have to be related to the search engine URL. The result URLs may be closer.*.state.search.availableEngines
? Otherwise you will not find a search engine if the user decided to hide it.appendSearchAttributionToUrlIfNeeded()
we already change the URL to the one with the search attribution, so we already know the context, it seems complicated to then check this here again. 🤔 This would also allow us to directly lookup the search engine (e.g. Google) and not have to search for it here, I assume?┆Issue is synchronized with this Jira Task
The text was updated successfully, but these errors were encountered: