Closed Bug 1402529 Opened 2 years ago Closed 2 years ago

Fix broken story dismissing, unintended telemetry and bug fixes to Activity Stream

Categories

(Firefox :: New Tab Page, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 58
Tracking Status
firefox57 --- fixed
firefox58 --- fixed

People

(Reporter: Mardak, Assigned: Mardak)

References

Details

(Whiteboard: [export])

User Story

https://github.com/mozilla/activity-stream/compare/2d88ef77af1855e3d0d1681758cca1413a94d2e8...2ee94db4d65174977af01707a378b8a6b33dd8d6

Attachments

(1 file)

No description provided.
Depends on: 1401683
Comment on attachment 8911412 [details]
Bug 1402529 - Fix broken story dismissing, unintended telemetry and bug fixes to Activity Stream.

https://reviewboard.mozilla.org/r/182878/#review188100

r=dmose from code inspection (i.e. not manually tested)
Attachment #8911412 - Flags: review?(dmose) → review+
Pushed by edilee@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/7f3101566d6f
Fix broken story dismissing, unintended telemetry and bug fixes to Activity Stream. r=dmose
https://hg.mozilla.org/mozilla-central/rev/7f3101566d6f
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 58
Comment on attachment 8911412 [details]
Bug 1402529 - Fix broken story dismissing, unintended telemetry and bug fixes to Activity Stream.

Approval Request Comment
[Feature/Bug causing the regression]: Pocket section broke with the previous export bug 1401683
[User impact if declined]: Broken UI when user tries to dismiss a story where it briefly disappears and comes back right away. Also, impression telemetry was wrongly being sent for certain types of user interactions for Highlights.
[Is this code covered by automated tests?]: Yes, Activity Stream has 100% line coverage
[Has the fix been verified in Nightly?]: Yes, 20170923100045
[Needs manual test from QE? If yes, steps to reproduce]: No
[List of other uplifts needed for the feature/fix]: none
[Is the change risky?]: Not really
[Why is the change risky/not risky?]: The primary change was just adding a condition to not do stuff at the wrong time (don't dismiss, don't send telemetry)
[String changes made/needed]: Localized strings were added
Attachment #8911412 - Flags: approval-mozilla-beta?
Assignee: nobody → edilee
Francesco, do you sign off on the l10n changes? Thanks
Flags: needinfo?(francesco.lodolo)
(In reply to Sylvestre Ledru [:sylvestre] from comment #6)
> Francesco, do you sign off on the l10n changes? Thanks

These specific strings are not exposed for localization via mozilla-central, so we're good.
Flags: needinfo?(francesco.lodolo)
Comment on attachment 8911412 [details]
Bug 1402529 - Fix broken story dismissing, unintended telemetry and bug fixes to Activity Stream.

Fix a bad UX. Taking it.
Should be in 57b3
Attachment #8911412 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
(In reply to Ed Lee :Mardak from comment #5)
> [Is this code covered by automated tests?]: Yes, Activity Stream has 100%
> line coverage
> [Has the fix been verified in Nightly?]: Yes, 20170923100045
> [Needs manual test from QE? If yes, steps to reproduce]: No

Setting qe-verify- based on Ed Lee's assessment on manual testing needs and the fact that this fix has automated coverage.
Flags: qe-verify-
Blocks: 1403215
Whiteboard: [export]
Component: Activity Streams: Newtab → New Tab Page
You need to log in before you can comment on or make changes to this bug.