Closed Bug 1593258 Opened 4 years ago Closed 4 years ago

Celebration Toast banner is not displayed when using a dirty or an older profile.

Categories

(Firefox :: Protections UI, defect, P1)

defect

Tracking

()

VERIFIED FIXED
Firefox 72
Tracking Status
firefox71 + verified
firefox72 --- verified

People

(Reporter: obotisan, Assigned: ewright)

References

Details

Attachments

(2 files)

Affected versions

  • Firefo x72.0a1
  • Firefox 70.0b6

Affected platforms

  • Windows 10 x64
  • macOS 10.13
  • Ubuntu 18.04 x64

Prerequisites

  • Firefox is launched with this dirty profile by using the command --allow-downgrade in terminal.
  • Set "browser.contentblocking.cfr-milestone.milestones" to [280, 5000, 10000, 25000, 50000, 100000, 500000]
  • "browser.contentblocking.cfr-milestone.milestone-achieved = 0” - Ensure this number is a smaller number than the milestone you wish to hit (it defaults to 0, so best to not change this pref at all)
  • Set "browser.newtabpage.activity-stream.asrouter.providers.cfr" to {"id":"cfr","enabled":true,"type":"local","localProvider":"CFRMessageProvider","frequency":{"custom":[{"period":"daily","cap":10}]},"categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":3600000}
  • Set "browser.contentblocking.cfr-milestone.update-interval" to 10

Steps to reproduce

  1. Visit http://reddit.com/ in another tab, and refresh the website until the blocked trackers hit the first millstone i.e "280".

Expected result

  • The celebration toast banner is displayed below the address bar: "Nightly/Firefox blocked over 280 trackers since <Month Year>! See All >"

Actual result

  • The celebration toast banner is not displayed.

Regression range

  • This is not a regression.

Additional notes

  • I tried the steps by using a profile created at least two months ago and issue is still reproducing.
Priority: -- → P1

I tested this with the provided profile and it behaved as described - I'd like to test with some other "dirty" profiles. I can't find an obvious reason why this is happening.

Assignee: nobody → ewright
Status: NEW → ASSIGNED

Were we able to reproduce?

Flags: needinfo?(ewright)

(In reply to Tony Cinotto [:tcinotto] (UTC-8) from comment #2)

Were we able to reproduce?

unfortunately yes it reproduces reliably, I'm still trying to debug

Flags: needinfo?(ewright)

Do we consider this a blocker?

Flags: needinfo?(chsiang)

I've looked into this further, it is an issue, but not exactly the issue reported here, I believe it doesn't have anything to do with the profile being old.

@Oana, can you test again by first navigating to 3+ pages, then edit "browser.contentblocking.cfr-milestone.milestone-achieved = 0". At this point the milestone should show on the next page navigation.

What's happening is that we are setting a flag to trigger the milestone, but the CFR message requires 4+ page navigations before it will show. After those navigations have been completed, we don't have the trigger anymore. I need to come up with a different way to make the trigger happen. This is fixable though, will report back soon.

Flags: needinfo?(oana.botisan)

I followed the steps from comment 5 on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64 using Latest Nightly.
The celebration toast banner is displayed after you set the pref browser.contentblocking.cfr-milestone.milestone-achieved to 0 and navigate to another page with trackers on it.
I would like to mention that on Windows and macOS, sometimes is no need to reset the pref in order for the banner to show, but on Ubuntu it doesn't.

Flags: needinfo?(oana.botisan)
Pushed by ewright@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2cadb540632e
set milestone achieved flag only when milestone shown, to allow it to trigger the show message again. r=k88hudson,nhnt11
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 72

I verified the fix with the last Nightly on Windows 10 x64, macOS 10.13 and Ubuntu 18.04 x64. The bug is not reproducing anymore.

Status: RESOLVED → VERIFIED

Please nominate this for Beta approval when you get a chance.

Flags: needinfo?(ewright)

Comment on attachment 9108277 [details]
Bug 1593258 - set milestone achieved flag only when milestone shown, to allow it to trigger the show message again.

Beta/Release Uplift Approval Request

  • User impact if declined: Users will not see the milestone message if the milestone is reached upon closing the browser, or reached in the first 4 page loads.
  • Is this code covered by automated tests?: Yes
  • Has the fix been verified in Nightly?: Yes
  • Needs manual test from QE?: No
  • If yes, steps to reproduce:
  • List of other uplifts needed: None
  • Risk to taking this patch: Low
  • Why is the change risky/not risky? (and alternatives if risky): Low because the only code affected by this patch is contained to the milestone. A pref was being set too early, now it is being set at the appropriate time.
  • String changes made/needed: none
Flags: needinfo?(ewright)
Attachment #9108277 - Flags: approval-mozilla-beta?

Comment on attachment 9108277 [details]
Bug 1593258 - set milestone achieved flag only when milestone shown, to allow it to trigger the show message again.

Low risk and a feature blocker, has tests and QA verified it in Nightly, uplift approved for 71 beta 12, thanks.

Attachment #9108277 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Uplifting to beta gave a conflict:

warning: conflicts while merging toolkit/components/antitracking/test/xpcshell/test_tracking_db_service.js! (edit, then use 'hg resolve --mark')

Flags: needinfo?(ewright)

I see, there's some changes added to that test file in Bug 1584479, and I added on top of them, will edit to work around those changes.

Flags: needinfo?(ewright)
Attached patch 1593258.diffSplinter Review

Approval Request Comment
Same as above.

This is the patch which should be applied to Beta. Identical to the above patch but with changes to toolkit/components/antitracking/test/xpcshell/test_tracking_db_service.js to no longer use the cleanup function added in a recent commit.

ni: nihanth to double check.

Flags: needinfo?(chsiang) → needinfo?(nhnt11)
Attachment #9110634 - Flags: approval-mozilla-beta?
Comment on attachment 9110634 [details] [diff] [review]
1593258.diff

I checked that this rebased patch correctly sets and resets the relevant prefs within the task, as opposed to the more test-global approach that's currently in Nightly.
Flags: needinfo?(nhnt11)
Attachment #9110634 - Flags: review+
Comment on attachment 9110634 [details] [diff] [review]
1593258.diff

Approving rebased patch or the beta branch  before RC.
Attachment #9110634 - Flags: approval-mozilla-beta? → approval-mozilla-beta+

Comment on attachment 9108277 [details]
Bug 1593258 - set milestone achieved flag only when milestone shown, to allow it to trigger the show message again.

Removing approval on the previous patch.

Attachment #9108277 - Flags: approval-mozilla-beta+ → approval-mozilla-beta-

We verified the fix using Firefox 71.0 - build 2 on Windows 10 and the bug is still reproducing.
Also, the pref browser.contentblocking.cfr-milestone.milestone-achieved still remains at 0 even if the milestone is reached.

Flags: needinfo?(ewright)

I tested this on 71 beta and it works as expected for me.
str:
open an old profile in 71
set the prefs:

  • browser.contentblocking.cfr-milestone.milestones = [10, 5000, 10000, 25000, 50000, 100000, 500000]
  • browser.contentblocking.cfr-milestone.update-interval = 10

visit 4+ pages
ex: https://senglehardt.com/test/trackingprotection/test_pages/tracking_protection.html

browser.contentblocking.cfr-milestone.milestone-achieved will now remain unchanged until the milestone message actually shows - this is by design and is the change implemented in this bug.

Flags: needinfo?(ewright) → needinfo?(oana.botisan)

I tried again with Firefox 71.0 - RC on Windows 10 x64. I can still reproduce the issue.
I will write the steps I did, because maybe we are doing them differently.

  1. Open Fx with a dirty profile (the profile from comment 0) by using the command --allow-downgrade.
  2. set prefs:
    - browser.contentblocking.cfr-milestone.milestones = [x, 5000, 10000, 25000, 50000, 100000, 500000] (where x is a number higher that the total trackers that are already blocked on the profile until this point)
    - browser.contentblocking.cfr-milestone.update-interval = 10
  3. Open 4 pages that have trackers and close them.
  4. Open another page that has trackers.

This is the point where the doorhanger should be displayed, but it's not.
Also, like I mentioned the pref browser.contentblocking.cfr-milestone.milestone-achieved still remains at 0 even if the milestone is reached.

Another things is that the trackers from the tests pages we used until this point are not detected anymore. Maybe I should log a different bug for that issue.

Flags: needinfo?(oana.botisan) → needinfo?(ewright)

I tested it with the linked profile, and I'm seeing the following errors (in conjunction with no messages showing up in the about:newtab#devtools):

In kinto-offline-client.js:

IndexedDB getLastModified() The operation failed for reasons unrelated to the database itself and not covered by any other error code.

ASRouter.jsm is also reporting:

UnknownError: The operation failed for reasons unrelated to the database itself and not covered by any other error code. ASRouter.jsm:138
    reportError resource://activity-stream/lib/ASRouter.jsm:138
    _remoteLoaderCache resource://activity-stream/lib/ASRouter.jsm:169
    InterpretGeneratorResume self-hosted:1152
    AsyncFunctionThrow self-hosted:693

To me, this looks like indexedDB failures caused by a corrupt database loaded with the profile, which I think could very possibly result from a downgrade. We're unable to load any messages if indexedDB / remote settings doesn't work.

Flags: needinfo?(ewright)

We tried with a different dirty profile and the issue is not reproducing anymore. We tested on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.13 using Firefox 71.0.
But I have to mention that we still had to change the pref "browser.newtabpage.activity-stream.asrouter.providers.cfr" to {"id":"cfr","enabled":true,"type":"local","localProvider":"CFRMessageProvider","frequency":{"custom":[{"period":"daily","cap":10}]},"categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":3600000}
I am not sure if this is an issue or not, considering the fact that we shouldn't have to change it anymore. Is that expected?

(In reply to Oana Botisan, Desktop Release QA from comment #25)

We tried with a different dirty profile and the issue is not reproducing anymore. We tested on Windows 10 x64, Ubuntu 18.04 x64 and macOS 10.13 using Firefox 71.0.
But I have to mention that we still had to change the pref "browser.newtabpage.activity-stream.asrouter.providers.cfr" to {"id":"cfr","enabled":true,"type":"local","localProvider":"CFRMessageProvider","frequency":{"custom":[{"period":"daily","cap":10}]},"categories":["cfrAddons","cfrFeatures"],"updateCycleInMs":3600000}
I am not sure if this is an issue or not, considering the fact that we shouldn't have to change it anymore. Is that expected?

The milestone has a lifetime maximum of 7, it will never naturally show more than 7 times (you will still be able to trigger it from dev tools). I believe this is why it is no longer showing, and why you would need to change the providers pref. I've been able to trigger it to show on an older profile without changing this pref.

According to comment 26 and comment 25 I will mark this bug as verified on Firefox 71. Thank you for the explanation, Erica.

You need to log in before you can comment on or make changes to this bug.