1795037 - "try it" doorhanger after closing a tab reappears frequently when also using private browsing

Status: VERIFIED FIXED

(Firefox :: Messaging System, defect, P1)

Description

[Chris H-C :chutten]

I gave Firefox View a try on my personal Nightly a week or two ago and didn't find it fit my workflow, so I turned it off using the UI. Since then, I've received multiple "You can find your recently-closed tabs in Firefox View [try it] [nah]" doorhangers after closing tabs.

Frequency: not every day, but definitely more than once a week. Perhaps once per build?

Is this UI operating as designed+desired and I'm a weirdo hitting a corner case? Or is this not desired behaviour (and you'd like some logs, environment info)?

Comment 1

[Ray Fambro]

Adding Venetia for visibility here.

Comment 2

[Punam Dahiya [:pdahiya]]

Hi Chris,
To help debug the issue can you please share the impression count of the doorhanger message. You can see it in devtools by following below steps

a) Set pref 'browser.newtabpage.activity-stream.asrouter.devtoolsEnabled' to true in about:config
b) Open about:newtab#devtools
c) Search for “CFR_FIREFOX_VIEW” , it should have impressions count respective to message

Also, will be great if you can share value of pref browser.firefox-view.view-count for the profile that shows the issue. Thanks!

Comment 3

[Chris H-C :chutten]

CFR_FIREFOX_VIEW is at 0 impressions. browser.firefox-view.view-count is at 0.

Comment 4

[Ed Lee :Mardak]

(In reply to Chris H-C :chutten from comment #0)

turned it off using the UI

This is "Remove from Toolbar" ? Other than the impression record seeming to be missing, potentially we should target on whether the button is in the toolbar in the first place?

Comment 5

[Chris H-C :chutten]

I believe it was "Remove from Toolbar". It was over a week ago and I didn't pay attention to it at the time, so my memory's fuzzy. I've also had to remove it from each computer and each variant of Firefox, so it's possible that I didn't actually impress on Firefox View on that profile before I removed it (though I could've sworn I had evaluated it on that profile).

It is present in Customize for re-adding, if that helps.

Comment 6

[Ed Lee :Mardak]

Attachment: example without firefox view button in toolbar

Comment 7

[Ed Lee :Mardak]

From some debugging with chutten, we saw that groupImpressions for "cfr" indeed recorded the impression timestamp from just before this bug was filed, but messageImpressions did not contain any timestamps for CFR_FIREFOX_VIEW. However, when we cleared the group impressions (which prevents another cfr from showing within 24 hours), closing 3 tabs correctly recorded the message impression.

So it seems like impression recording behavior for groups and messages is working, but for some reason the message impression gets cleaned up perhaps via cleanupImpressions:
https://searchfox.org/mozilla-central/rev/76ccfc801e6b736c844cde3fddeab7a748fc8515/browser/components/newtab/lib/ASRouter.jsm#1365-1381

That method should only remove impressions for messages it no longer knows about, so potentially CFR_FIREFOX_VIEW is somehow is forgotten in at least one of the many times messages are loaded from providers?

Comment 8

[Chris H-C :chutten]

As promised, here I am over 86400s later reporting that I have not seen the doorhanger today, even upon closing masses of tabs. My messageImpressions per about:newtab#devtools does not have a firefox view CFR (it does have the tcp-cfr-thank-you we played with yesterday). I have 0 group impressions.

Ah-ha, and then I updated to a new build, loaded a bunch of tabs, then closed three of them and got the message. I skipped out on focus by clicking over to this Firefox to report on it. Now my group impressions are at 1 for cfr and I do have an impression for CFR_FIREFOX_VIEW in my messageImpressions. We'll see if the impression disappears by Monday, and if the cfr comes back.

Comment 9

[Punam Dahiya [:pdahiya]]

(In reply to Chris H-C :chutten from comment #8)

As promised, here I am over 86400s later reporting that I have not seen the doorhanger today, even upon closing masses of tabs. My messageImpressions per about:newtab#devtools does not have a firefox view CFR (it does have the tcp-cfr-thank-you we played with yesterday). I have 0 group impressions.

Ah-ha, and then I updated to a new build, loaded a bunch of tabs, then closed three of them and got the message. I skipped out on focus by clicking over to this Firefox to report on it. Now my group impressions are at 1 for cfr and I do have an impression for CFR_FIREFOX_VIEW in my messageImpressions. We'll see if the impression disappears by Monday, and if the cfr comes back.

Thanks @chutten, trying to replicate at my end by updating Nightly build but haven't seen impression disappear. Please do share your findings and impression count for CFR_FIREFOX_VIEW on Monday and also before you update to a new build.

Comment 10

[Chris H-C :chutten]

Alas I missed the "and also before you update to a new build" and so only have information from after. No message this time.

0 cfr group impression, but 1 persistent CFR_FIREFOX_VIEW view count per newtab devtools. Looks like whatever was happening might've gotten scared when we poked it so it hasn't happened again. We maybe should close this as invalid/incomplete for now. I'll find it and reopen if it happens again.

Comment 11

[Chris H-C :chutten]

And it happened again today. I installed an update and restarted via the "an update's here, restart?" flow and afterwards closed three tabs. CFR_FIREFOX_VIEW is at 1 impression despite it having been at 1 impression 2 days ago (per comment 10) and me getting one more impression today.

Definitely fuel for the "something is clearing impressions" theory.

Comment 12

[Chris H-C :chutten]

Ah-ha! Caught it.

Before I updated this morning, I checked about:newtab#devtools and CFR_FIREFOX_VIEW was not even present in the list of possible messages! Then I updated, and CFR_FIREFOX_VIEW was back. And of course it's impression count was 0.

So it might not be that something is cleaning up the impressions, it might be that something is cleaning up the messages.

Comment 13

[Punam Dahiya [:pdahiya]]

(In reply to Chris H-C :chutten from comment #12)

Ah-ha! Caught it.

Before I updated this morning, I checked about:newtab#devtools and CFR_FIREFOX_VIEW was not even present in the list of possible messages! Then I updated, and CFR_FIREFOX_VIEW was back. And of course it's impression count was 0.

So it might not be that something is cleaning up the impressions, it might be that something is cleaning up the messages.

that's correct, I believe you were able to see about:newtab#devtools at the time when ASRouter internal state was missing some messages which leads to clean up of missing messages impressions, any chance you have screen shot of what all messages were available/shown at that time in devtools , will help in narrowing down the flow that's causing ASRouter messages state to change and drop messages thank you!

Comment 14

[Dan Mosedale (:dmosedale, :dmose)]

A thing that might help in figuring out what's going on here is to create a string preference in about:confignamed browser.newtabpage.activity-stream.asrouter.debugLogLevel with a value of all. This will cause ASRouter to log a bunch of stuff to the browser console, including a stack trace for every call to ASRouter.setState, so that if you catch it in the act again, the browser console could contain some useful evidence.

Comment 15

[Punam Dahiya [:pdahiya]]

Noticed opening about:privatebrowsing does update ASRouter message state to PrivateBrowsing Messages, trying to replicate now what and duration of window that causes cleanup of Impressions

Comment 16

[Punam Dahiya [:pdahiya]]

Here are the steps of one of the flow using which I was able to reliably replicate cleanup of impressions for CFR_FIREFOX_VIEW message

1. On a new profile, Open about:newtab#devtools -> Click show button respective to CFR_FIREFOX_VIEW -> Dismiss CFR by clicking Not Now
2. Reload about:newtab#devtools -> Targeting -> Messaging Impressions under targeting should CFR_FIREFOX_VIEW messsage impression
3. Open about:privatebrowsing which shows Pin or Focus promo by default , message impression of PB_NEWTAB_FOCUS_PROMO or pin promo should get added to message impression object under Targeting in step 2
4. Close about:privatebrowsing and leave Firefox instance open for an hour, which allows shouldProviderUpdate to return true for a provider which checks if the time since .lastUpdated is greater than .updateCycleInMs (1hr)

https://searchfox.org/mozilla-central/source/browser/app/profile/firefox.js#1543

5. Open a new tab or click on Firefox view icon to trigger 'defaultBrowserCheck' or 'featureCalloutCheck' that calls loadMessagesFromAllProviders, with cleanupImpressions call inside

https://searchfox.org/mozilla-central/rev/12a18f7e112a4dcf88d8441d439b84144bfbe9a3/browser/components/newtab/lib/ASRouter.jsm#829
https://searchfox.org/mozilla-central/rev/12a18f7e112a4dcf88d8441d439b84144bfbe9a3/browser/components/newtab/lib/ASRouter.jsm#833
https://searchfox.org/mozilla-central/rev/12a18f7e112a4dcf88d8441d439b84144bfbe9a3/browser/components/newtab/lib/ASRouter.jsm#449
https://searchfox.org/mozilla-central/rev/12a18f7e112a4dcf88d8441d439b84144bfbe9a3/browser/components/newtab/lib/ASRouter.jsm#885

5. In Devtools ->Targeting -> Messaging Impressions, CFR_FIREFOX_VIEW impression should be missing

The reason for above cleanup is in about:privatebrowsing, ASRouter keeps PrivateBrowsing messages only , I will submit the patch that fix filtering out of non-PBM messages keeping ASRouter state consistent with regular browsing

https://searchfox.org/mozilla-central/rev/12a18f7e112a4dcf88d8441d439b84144bfbe9a3/browser/components/newtab/lib/ASRouter.jsm#1755

Comment 17

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

Set release status flags based on info from the regressing bug 1779858

Comment 18

[Punam Dahiya [:pdahiya]]

Attachment: Bug 1795037 - Fix filter of messages in ASRouter state after using PBM

Comment 19

[Donal Meehan [:dmeehan]]

Looks like it's too late for 107, the WIP patch doesn't look like something we want to uplift.
Do we plan on addressing this for 108? Could a severity be set?

Comment 20

[Punam Dahiya [:pdahiya]]

[Tracking Requested - why for this release]:
The issue could be quite annoying for users who visits about:privatebrowsing and ends up seeing CFR prompt. Also, fix in this patch will help fix related issues seen for callout in Bug 1794261 and Bug 1794849.

Comment 21

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/e22a388668c5
Fix filter of messages in ASRouter state after using PBM r=Mardak

Comment 22

[Donal Meehan [:dmeehan]]

(In reply to Punam Dahiya [:pdahiya] from comment #20)

[Tracking Requested - why for this release]:
The issue could be quite annoying for users who visits about:privatebrowsing and ends up seeing CFR prompt. Also, fix in this patch will help fix related issues seen for callout in Bug 1794261 and Bug 1794849.

I've changed 107 status and to tracking +. Thanks for setting the severity and landing the patch.
Please add a beta uplift request when ready.

Comment 23

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

The bug is marked as tracked for firefox107 (beta). However, the bug still has low severity.

:tspurway, could you please increase the severity for this tracked bug? If you disagree with the tracking decision, please talk with the release managers.

For more information, please visit auto_nag documentation.

Comment 24

[Sandor Molnar[:smolnar]]

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

Comment 25

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

The patch landed in nightly and beta is affected.
:pdahiya, is this bug important enough to require an uplift?

• If yes, please nominate the patch for beta approval.
• If no, please set status-firefox107 to wontfix.

For more information, please visit auto_nag documentation.

Comment 26

[Punam Dahiya [:pdahiya]]

Comment on attachment 9299993 [details]
Bug 1795037 - Fix filter of messages in ASRouter state after using PBM

Beta/Release Uplift Approval Request

• User impact if declined: User can see annoying CFR prompt and repeat colorways/tab firefox view reminders due to missing impression
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: Yes
• If yes, steps to reproduce: See testplan in https://phabricator.services.mozilla.com/D160125
• List of other uplifts needed: none
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Small patch that only touches message filter for private browsing mode
• String changes made/needed: none
• Is Android affected?: No

Comment 27

[Donal Meehan [:dmeehan]]

Comment on attachment 9299993 [details]
Bug 1795037 - Fix filter of messages in ASRouter state after using PBM

Approved for 107.0b6.

Comment 28

[Donal Meehan [:dmeehan]]

https://hg.mozilla.org/releases/mozilla-beta/rev/3cc8427ca4fb

Comment 29

[Simona Rosu [:srosu], Ecosystem QA]

I‘ve verified this issue using the latest Firefox Nightly 108.0a1 (Build ID: 20221027095047) on Windows 10 x64.

• After following the steps described in the Test Plan, I can confirm that the "See how it works" Firefox View spotlight is displayed on the "Firefox View" page after closing about:privatebrowsing where the Focus/Pin Promo message was displayed.

Comment 30

[Simona Rosu [:srosu], Ecosystem QA]

I‘ve verified this issue using Firefox Beta 107.0b6 (Build ID: 20221027185833) on Windows 10 x64, macOS 12.6, and Ubuntu 20.04 x64.

• After following the steps described in the Test Plan, I can confirm that the "See how it works" Firefox View spotlight is displayed on the "Firefox View" page after closing about:privatebrowsing where the Focus/Pin Promo message was displayed.