Closed Bug 1815710 Opened 3 years ago Closed 3 years ago

Retarget onboarding doorhanger to avoid search doorhanger conflict

Categories

(Core :: Privacy: Anti-Tracking, defect)

defect

Tracking

()

VERIFIED FIXED
Tracking Status
firefox112 --- verified

People

(Reporter: jhirsch, Assigned: jteow)

References

Details

(Whiteboard: [snt-scrubbed])

We discovered in 111 Nightly that there is a search onboarding doorhanger shown when the user types a search in the address bar. This can conflict with the CBH onboarding doorhanger. We need to find a way to avoid the conflict.

Assignee: nobody → jhirsch
Status: NEW → ASSIGNED

A possible solution here is to add the CBH panel's anchor (the shield panel toolbar button, I think but need to confirm) to the link at [4]. But I need to verify this works locally before we go forward with it.


Background info from :jteow on the search team:

How the targeting works for the search doorhanger:

To always show the search tip, you can set the pref to: browser.urlbar.searchTips.test.ignoreShowLimits: true
For the persist search terms feature, it’s gated by a boolean pref browser.urlbar.showSearchTerms.featureGate which will be flipped by Nimbus, and browser.urlbar.showSearchTerms.enabled is the other boolean pref that will actually enable/disable the feature. When the feature is available for a user to use, they also have the ability to disable the persist feature in about:preferences#search under the image of the urlbar only navigation.

Relevant code paths:

We’re using the Search Tips Provider[1] to show the message. We have a condition that checks[2] if the Persist Search Terms feature is enabled, doesn’t exceed the max number of times a search tip is shown, and is on a default SERP. We already do some logic[3] as to when not to show a search tip if another notification is showing.

[1] https://searchfox.org/mozilla-central/rev/e7dd9d9ba128478e1ff399778e851365ced3c806/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs
[2] https://searchfox.org/mozilla-central/rev/e7dd9d9ba128478e1ff399778e851365ced3c806/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs#407-414
[3] https://searchfox.org/mozilla-central/rev/e7dd9d9ba128478e1ff399778e851365ced3c806/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs#480-485
[4] https://searchfox.org/mozilla-central/rev/e7dd9d9ba128478e1ff399778e851365ced3c806/browser/components/urlbar/UrlbarProviderSearchTips.sys.mjs#520

After doing some digging, I think we are onto a fix. Update from jteow:

I feel like checking if PopupNotifications.isPanelOpen should be enough. Just checking, yeah it’s true when the Cookie Popup shows (I enabled the feature and went to coolblue.nl ). Still have to look into why though the tip is still firing despite the previous check that should’ve worked.

It looks like jteow is going to work on a patch here in the urlbar code

:jteow, would it be useful to assign this bug to you? if you're planning to fix a separate bug in the search / urlbar components instead, I can just dupe this one to yours.

Flags: needinfo?(jteow)

Yup, I'll just it assign it to myself.

Assignee: jhirsch → jteow
Flags: needinfo?(jteow)
Whiteboard: [snt-scrubbed]

I'm working on fixing Bug 1812232 which should fix this bug.

The weird flickering that :jhirsch recorded is very odd though. When a PopupNotification is anchored to an element that doesn't exist, the component code has it attached to a guaranteed fallback element, so I'm not sure why in this particular case it does crazy flickering and loops.

See Also: → 1812232

FYI :jhirsch, the fix for Bug 1812232 is on Central. I followed STR the CBH PopupNotification on my end and it seemed to show up properly.

I'll keep this open and you can resolve this as fixed if you can confirm that you no longer see an issue, as the flickering you saw is quite different from merely not seeing the proper Geo icon in Bug 1812232. The solution though is essentially the same.

I'll do some investigation on the side to determine why the construction of your popup didn't play well with the Urlbar when icons weren't present because it should've failed gracefully like the Geo popup instead of get stuck in a loop of showing and hiding. It is a symptom of something problematic.

Flags: needinfo?(jhirsch)

Sorry for the lack of movement here--I confirmed in local testing that this issue was no longer reproducible. I think we're good to close, but I'll turn this over to :ppop, who is providing QA for cookie banner handling, to double check that this bug is fixed.

Flags: needinfo?(jhirsch) → needinfo?(ppop)

I've verified that this issue is no longer reproducible, the doorhanger are displayed one after the other without conflict. Tested using Firefox Nightly Version 112.0a1, Build ID 20230215093902 on Windows 10 x64, macOS 12.5, and Ubuntu 22.04 x64. I will mark this bug as verified.

Status: ASSIGNED → RESOLVED
Closed: 3 years ago
Flags: needinfo?(ppop)
Resolution: --- → FIXED
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.