Impressions on spocs for hero regression
Categories
(Firefox :: New Tab Page, defect, P1)
Tracking
()
Tracking | Status | |
---|---|---|
firefox-esr60 | --- | unaffected |
firefox65 | --- | unaffected |
firefox66 | + | fixed |
firefox67 | --- | verified |
People
(Reporter: thecount, Assigned: thecount)
References
Details
(Keywords: github-merged, regression)
Attachments
(4 files, 1 obsolete file)
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-github-pull-request
|
Details | |
(deleted),
text/x-phabricator-request
|
lizzard
:
approval-mozilla-beta+
|
Details |
(deleted),
image/png
|
Details |
Assignee | ||
Updated•5 years ago
|
Comment 1•5 years ago
|
||
Assignee | ||
Comment 2•5 years ago
|
||
[Tracking Requested - why for this release]:
Assignee | ||
Comment 3•5 years ago
|
||
To test:
- Set
browser.newtabpage.activity-stream.discoverystream.config
to{"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=dev-test-all"}
- Scroll down until you can no longer see "Hero (with borders, 5 items, with a Spoc)"
- Check
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
- Go back to the original about:home page from step 2.
- Refresh it a few times.
- The value in
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
should now have grownn with more impressions.
Updated•5 years ago
|
Comment 4•5 years ago
|
||
Comment 5•5 years ago
|
||
Updated•5 years ago
|
Comment 6•5 years ago
|
||
Assignee | ||
Comment 7•5 years ago
|
||
Assignee | ||
Comment 8•5 years ago
|
||
Comment on attachment 9046252 [details]
Bug 1529730 - Impression capping for spocs on hero
Beta/Release Uplift Approval Request
- Feature/Bug causing the regression: Bug 1522832
- User impact if declined: Experiments (Would do weird things to the results because user would see spocs too often/long than the control in a way we are not testing)
user experience (User don't expect to see spocs this often/long) - Is this code covered by automated tests?: No
- Has the fix been verified in Nightly?: No
- Needs manual test from QE?: Yes
- If yes, steps to reproduce: 1. Set
browser.newtabpage.activity-stream.discoverystream.config
to{"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=dev-test-all"}
- Scroll down until you can no longer see "Hero (with borders, 5 items, with a Spoc)"
- Check
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
- Go back to the original about:home page from step 2.
- Refresh it a few times.
- The value in
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
should now have grownn with more impressions.
- List of other uplifts needed: None
- Risk to taking this patch: Medium
- Why is the change risky/not risky? (and alternatives if risky): It's 1 line, which would normally have me click low risk on this.
But given the time, and the fact that I'm getting this request out there while still waiting on QA to verify on nightly and the phabricator patch to be reviewed. Once those to things are done this is a def low risk patch, so I'm happy to wait for those but wasn't sure if waiting or sending the request now would be better.
- String changes made/needed: None
Comment on attachment 9046252 [details]
Bug 1529730 - Impression capping for spocs on hero
OK, let's uplift for beta 11.
Updated•5 years ago
|
Assignee | ||
Comment 10•5 years ago
|
||
QA verification for nightly needed.
- Set
browser.newtabpage.activity-stream.discoverystream.config
to{"api_key_pref":"extensions.pocket.oAuthConsumerKey","enabled":true,"show_spocs":true,"layout_endpoint":"https://getpocket.com/v3/newtab/layout?version=1&consumer_key=$apiKey&layout_variant=dev-test-all"}
- Scroll down until you can no longer see "Hero (with borders, 5 items, with a Spoc)"
- Check
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
- Go back to the original about:home page from step 2.
- Refresh it a few times.
- The value in
browser.newtabpage.activity-stream.discoverystream.spoc.impressions
should now have grownn with more impressions.
Feel free to ping me with any issues verifying it.
Updated•5 years ago
|
Comment 11•5 years ago
|
||
Observation on Windows 10 Pro (VM)
I had basic
end-point initially - After I change it to dev-test-all
(per bug) , there is a slight delay on loading new tab --> since the new tab still has basic
’s spoc.. spoc.impressions
was able to capture time stamp of this --> later after new tab loaded with dev-test-all
layout --> spoc.impressions
continues to capture latest timestamps in addition to the basic
timestamp --> In general dev-test-all
doesn't have spoc on the first visibility of page (user need to scroll in order to see first spoc
)
I have a recording after initial timestamp (which is basic
layout's spoc) :
https://www.dropbox.com/s/i1jrfmyimive2we/hero%20spocs%20-%20bug%201529730.mp4?dl=0
Assignee | ||
Comment 12•5 years ago
|
||
Yup that all looks right to me. I counted along the spocs you see and the impressions.
In your video:
- It starts at 1 impression from basic.
- You scroll down dev-test-all and pass/view 2 more, total is 3.
- You scroll down a bit more and see 1 more. There is another almost visible, but it's not quite 50% displayed so it shouldn't count. Total is now 4.
- You refresh, and two are displayed. The one is fully displayed, and there is one at the top that is 50%+ in view. Total is now 6.
- You refresh the same page again without scrolling, so another +2 making total 8. The actual breakdown at this point is 788x3, 1286x3, and 1397x2.
- Refresh 1 more time, making the total 10 with two more view of 1397.
Step 5-6 shows an interesting edge case where you can 4 spoc impressions even though the limit is 3, because it only checks the limit when it first renders the page, not when it renders each spoc. This causes the page when it renders those 2 new spocs, they both render before you've seen them, thus they both happily render because they are both still at only 3 views. It's known unrelated issue that we've got a fix for in 67 here: https://bugzilla.mozilla.org/show_bug.cgi?id=1522899
Comment 13•5 years ago
|
||
Awesome, thanks for clarifying Scott. This is helpful.
Comment 14•5 years ago
|
||
This shall land on beta after t has been QA-verified. Is that correct?
Updated•5 years ago
|
Updated•5 years ago
|
Actually, I'm ok with this landing for beta 12.
It might need a little extra testing in beta once it lands.
Updated•5 years ago
|
Comment 16•5 years ago
|
||
QA Results:
Tested on :
FF Nightly version : 67.0a1 (2019-02-25)
OS : Mac and Windows 10 Pro
Works as expected.
QA Results - bug 1529730 (Mac OS).png
For Windows there's a recording in comment# 11.
Marking as Verified
Updated•5 years ago
|
Updated•5 years ago
|
Comment 17•5 years ago
|
||
bugherder uplift |
Comment 18•5 years ago
|
||
Brahmini Nagabandi, can you please verify this issue on latest Firefox 66 Beta 12(http://archive.mozilla.org/pub/firefox/candidates/66.0b12-candidates/build1/)
Updated•5 years ago
|
Comment 19•5 years ago
|
||
QA Results:
Tested on :
FF Beta version : 66.0b14 (64-bit)
OS : Mac and Windows 10 Pro
Verified on the latest Beta. Works as expected.
Marking as verified.
Updated•5 years ago
|
Description
•