1976706 - Replace ASRouter's use of `nsISearchEngine.identifier`

Status: RESOLVED FIXED

(Firefox :: Messaging System, task, P2)

Description

[Mark Banner (:standard8)]

In bug 1877721, we'd like to remove nsISearchEngine.identifier as it is ambiguous with what it is, and overlaps with id and telemetryId.

We believe ASRouter's use is unused at the moment (it was originally added in bug 1432673), so we can hopefully replace it whilst it is not used.

I think we can change the .identifier uses to .id - these are already filtered to only getAppProvidedEngines, so we don't need to worry about the non-application provided case. Changing to id will remove the telemetry suffix, but I think that is more consistent and an improvement. For example, Google would now report as google rather than google-b-d, google-b-1-d and other variations.

Comment 1

[Mimi Jung]

Hi Mark, we don't see any concerns with removing nsISearchEngine.identifier.

Comment 2

[Emily McMinn :emcminn]

Hi Mark! Is someone from your team intending to pick this up or would you like someone from OMC make the change?

Comment 3

[Mark Banner (:standard8)]

(In reply to Emily McMinn :emcminn from comment #2)

Hi Mark! Is someone from your team intending to pick this up or would you like someone from OMC make the change?

Either way - we are planning on making the change but it's been delayed due to some other work going on, though hopefully that'll happen in the next couple of days.

Comment 4

[Mark Banner (:standard8)]

Attachment: Bug 1976706 - Change ASRouterTargeting's searchEngines option to use the search engine id rather than the identifier. r?#omc-reviewers!

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://github.com/mozilla-firefox/firefox/commit/0662b4a8bfff
https://hg.mozilla.org/integration/autoland/rev/874c55032e3d
Change ASRouterTargeting's searchEngines option to use the search engine id rather than the identifier. r=omc-reviewers,pdahiya

Comment 6

[amarc]

https://hg.mozilla.org/mozilla-central/rev/874c55032e3d