1780564 - Glean default search engine data is reporting errors when submitting an empty submission URL

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Chris H-C :chutten]

Missed a case in bug 1775312. Specifically for private search engine's submission url, it's still being set to an invalid URL here:

    if (info.defaultPrivateSearchEngineData) {
      Glean.searchEnginePrivate.displayName.set(
        info.defaultPrivateSearchEngineData.name
      );
      Glean.searchEnginePrivate.loadPath.set(
        info.defaultPrivateSearchEngineData.loadPath
      );
      Glean.searchEnginePrivate.submissionUrl.set(
        info.defaultPrivateSearchEngineData.submissionURL ?? "blank:"
      );
      Glean.searchEnginePrivate.verified.set(
        info.defaultPrivateSearchEngineData.origin
      );
    } else {
      Glean.searchEnginePrivate.displayName.set("");
      Glean.searchEnginePrivate.loadPath.set("");
      Glean.searchEnginePrivate.submissionUrl.set(""); // <= HERE
      Glean.searchEnginePrivate.verified.set("");
    }

I shoulda caught it in review. Whoops!

Comment 1

[Chris H-C :chutten]

Attachment: Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

Glean assumes non-url-parseable values for url types are a mistake.

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/1702a9baea2e
Ensure the private search submission url isn't blank r=Standard8

Comment 3

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/1702a9baea2e

Comment 4

[Mark Banner (:standard8)]

Comment on attachment 9286457 [details]
Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

ESR Uplift Approval Request

• If this is not a sec:{high,crit} bug, please state case for ESR consideration: Fixes a telemetry reporting issue with default search engines which is raising errors due to incorrect recording (follow-on from bug 1775312)
• User impact if declined: N/A. Fixes some new telemetry reporting
• Fix Landed on Version: 104
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Trivial change to handle the telemetry reporting better and avoid errors being reported.

Comment 5

[Dianna Smith [:diannaS]]

Comment on attachment 9286457 [details]
Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

Approved for 102.2esr

Comment 6

[Dianna Smith [:diannaS]]

Comment on attachment 9286457 [details]
Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

this needs a rebased patch for esr102

Comment 7

[Chris H-C :chutten]

Attachment: Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

Glean assumes non-url-parseable values for url types are a mistake.

Comment 8

[Chris H-C :chutten]

Wasn't sure how to properly attach the esr102 version of this, so here it is as a slightly different differential revision. Is this what you needed, Dianna?

Comment 9

[Dianna Smith [:diannaS]]

Comment on attachment 9289955 [details]
Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

Yes this grafted cleanly. It was complaining because SearchService.sys.mjs didn't exist in esr102.

Comment 10

[Dianna Smith [:diannaS]]

Comment on attachment 9289955 [details]
Bug 1780564 - Ensure the private search submission url isn't blank r?standard8!

Approved for 102.2esr

Comment 11

[Dianna Smith [:diannaS]]

https://hg.mozilla.org/releases/mozilla-esr102/rev/f0ebee0d17f6

Comment 12

[Adrian Florinescu [:aflorinescu]]

Confirmed that on 102.1.0(w/o fix) that glean.value.invalid_value is recorded for search.engine.private.submission_url in case of private engine enabled and verified on 102.2.0RCb2(w fix) that the value is recorded for search.engine.private.submission_url as "blank:" and no glean.value.invalid_value.
Verification done on Windows 10, Mac 11 and Ubuntu 22.

Will follow up Monday with the rest of channels.

Comment 13

[Adrian Florinescu [:aflorinescu]]

Followed up with verification on Windows 10, Mac 11 and Ubuntu 22. using 105.0a1 (2022-08-22) and 104.0 (64-bit) RC3