Celebration Toast banner is not displayed when using a dirty or an older profile.
Categories
(Firefox :: Protections UI, defect, P1)
Tracking
()
People
(Reporter: obotisan, Assigned: ewright)
References
Details
Attachments
(2 files)
47 bytes,
text/x-phabricator-request
|
pascalc
:
approval-mozilla-beta-
|
Details | Review |
9.08 KB,
patch
|
nhnt11
:
review+
pascalc
:
approval-mozilla-beta+
|
Details | Diff | Splinter Review |
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
- 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.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 1•4 years ago
|
||
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 | ||
Updated•4 years ago
|
Assignee | ||
Comment 3•4 years ago
|
||
(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
Assignee | ||
Comment 5•4 years ago
•
|
||
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.
Assignee | ||
Updated•4 years ago
|
Assignee | ||
Comment 6•4 years ago
|
||
Updated•4 years ago
|
Reporter | ||
Comment 7•4 years ago
|
||
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.
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
Comment 9•4 years ago
|
||
bugherder |
Reporter | ||
Comment 10•4 years ago
|
||
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.
Comment 11•4 years ago
|
||
Please nominate this for Beta approval when you get a chance.
Assignee | ||
Comment 12•4 years ago
|
||
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
Comment 13•4 years ago
|
||
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.
Comment 14•4 years ago
|
||
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')
Assignee | ||
Comment 15•4 years ago
|
||
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.
Assignee | ||
Comment 16•4 years ago
|
||
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.
Comment 17•4 years ago
|
||
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.
Comment 18•4 years ago
|
||
Comment on attachment 9110634 [details] [diff] [review] 1593258.diff Approving rebased patch or the beta branch before RC.
Comment 19•4 years ago
|
||
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.
Comment 20•4 years ago
|
||
bugherder uplift |
Reporter | ||
Comment 21•4 years ago
|
||
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.
Assignee | ||
Comment 22•4 years ago
|
||
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.
Reporter | ||
Comment 23•4 years ago
|
||
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.
- Open Fx with a dirty profile (the profile from comment 0) by using the command --allow-downgrade.
- 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 - Open 4 pages that have trackers and close them.
- 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.
Comment 24•4 years ago
|
||
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.
Assignee | ||
Updated•4 years ago
|
Reporter | ||
Comment 25•4 years ago
|
||
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?
Assignee | ||
Comment 26•4 years ago
|
||
(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.
Reporter | ||
Comment 27•4 years ago
|
||
According to comment 26 and comment 25 I will mark this bug as verified on Firefox 71. Thank you for the explanation, Erica.
Description
•