1759207 - Infobar not displayed (on restart) when Firefox set default engine is removed

Status: VERIFIED FIXED

(Firefox :: Search, defect, P1)

Description

[Adrian Florinescu [:aflorinescu]]

Description
When a default engine is removed by RS configuration update due contractual or other circumstances an infobar is expected to inform the user about the change. There are two ways to trigger this once the RS update is in the pipeline: restart or 5' idle. In case of f a Firefox set default search engine removal, restart won't trigger the infobar (5' idle time will)

Affected versions

• Firefox 98
• Beta 99
• Nightly 100

Affected platforms

• all

Steps to reproduce

1. Create a profile for a region/localization that has google.com as Firefox set default engine.
2. Have a search-config update that contains a removal of a Firefox set engine, e.g. google.com
3. Have the search-config update in pipeline. (changes are brought from RS, but not yet applied)
4. Restart browser. (RS changes are applied)

Expected result
Default engine is removed and Infobar is displayed stating that the google.com engine is replaced.

Actual result
Default engine is removed, but no Infobar is displayed in this case

Regression range

• Not a regression

Note:
This issue does not apply to user set default engine, but it applies to distribution set engines.

Comment 1

[Mandy Cheang [:mcheang][she/her]]

Attachment: Bug 1759207 - Display infobar when App provided default engine is removed on start-up. r?Standard8!,daleharvey

Comment 2

[Mandy Cheang [:mcheang][she/her]]

Attachment: Bug 1759207 - Add XPCShell test to show infobar for the removal of App provided default engine on start-up. r?Standard8,daleharvey

Depends on D142361

Comment 3

[Mandy Cheang [:mcheang][she/her]]

Attachment: Remove engine Infobar flow.jpg

Comment 4

[Adrian Florinescu [:aflorinescu]]

Hey Mandy, having a diagram makes a wold of difference, but can you dumb it down a little ? :) I'm trying to make some sense of it and the types of engines state confuse me.
Also, will this fix also account for the case of Live lang switching: bug 1761508?

Comment 5

[Mark Banner (:standard8)]

It might be worth drawing the diagram from the user's perspective, I think that might be easier to follow, and that it would be something I think we could include in the search documentation, which I think might be worth it for the future.

It might also be worth investigating if Mermaid can support this diagram (e.g. existing example) - though I've had mixed experiences with it in the past.

Comment 6

[Mandy Cheang [:mcheang][she/her]]

Hey Adrian and Mark, thank you for the feedback 🙂. I will make another diagram from the user's perspective. I got a little confused myself so this was my first draft of the diagram, but I see how the diagram can be improved to make it easier to understand and less specific about the variables in the code.

Mark, in regards to using Mermaid, I don't think it would be a good idea to use code to generate this diagram. The reason is because I initially used flowchart.js to create the diagram. The arrows became messy and hard to read very quickly. I am looking at Mermaid's code you've linked, it doesn't seem like there's an easy way to manipulate the arrows because the shapes and arrows are auto-generated. Many of the No outputs lead to Do not show infobar when I used flowchart.js, and many arrows pointing to Do not show info bar were crossing over each other and became hard to read. I suspect this same problem will occur if I were to use Mermaid for a big diagram like this. In the end, I redrew the diagram without flowchar.js and manually created it using https://app.diagrams.net/

Overall, I agree with having a newer version of the diagram and I can create a new patch to add that in the search documentation.

---

Re: comment 4.
Adrian, I don't think this fix will account for the case of Live lang switching: bug 1761508. At least I am not sure right now if it does but I can investigate it further. I can assign myself to bug 1761508

Comment 7

[Mandy Cheang [:mcheang][she/her]]

I just landed this patch. Thanks for the review, Mark.

I am planning to create a new diagram. It is still WIP at the moment. I am going to file another bug ticket for the diagram and put up a patch to be reviewed so we can add the diagram to the source docs.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/ed37c58f882a
Display infobar when App provided default engine is removed on start-up. r=Standard8

Comment 9

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/ed37c58f882a

Comment 10

[BugBot [:suhaib / :marco/ :calixte]]

Since the status are different for nightly and release, what's the status for beta?
For more information, please visit auto_nag documentation.

Comment 11

[Mandy Cheang [:mcheang][she/her]]

Comment on attachment 9269901 [details]
Bug 1759207 - Display infobar when App provided default engine is removed on start-up. r?Standard8!,daleharvey

Beta/Release Uplift Approval Request

• User impact if declined: If a default search engine is removed. Without this patch, it will not show the info bar to alert the user.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: No
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: Step to reproduce are same as the steps in comment 0. But with this fix, the infobar should be displayed.
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): There's automated test for this patch and I have tested this manually myself.
• String changes made/needed:
• Is Android affected?: Yes

Comment 12

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9269901 [details]
Bug 1759207 - Display infobar when App provided default engine is removed on start-up. r?Standard8!,daleharvey

Approved for 101.0b6.

Comment 13

[Ryan VanderMeulen [:RyanVM]]

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

Comment 14

[Adrian Florinescu [:aflorinescu]]

With the bug 1769570 logged as a follow-up, this issue has been verified* on Ubuntu, Windows 10 and Mac 11 using Beta 101.0b7 and Nightly 102.0a1 (2022-05-16).

• Environment used: staging/main-preview with google and bing temporarily removed.