Page MenuHomePhabricator
Create Task
Maniphest T220979

[Bug] Notification counts broken on mobile
Closed, ResolvedPublic
Actions

Assigned To
ovasileva
Authored By
• Niedzielski
Apr 15 2019, 2:00 PM
Tags
Referenced Files
F29790862: IMG_3312.PNG
Jul 17 2019, 1:12 PM
F29790864: IMG_3311.PNG
Jul 17 2019, 1:12 PM
F29790863: IMG_3313.PNG
Jul 17 2019, 1:12 PM
F28744618: en.wikipedia.org_wiki_Cat (2).png
Apr 23 2019, 2:27 PM
F28744591: en.wikipedia.org_wiki_Cat.png
Apr 23 2019, 2:27 PM
F28744595: en.m.wikipedia.org_wiki_Cat.png
Apr 23 2019, 2:27 PM
F28744612: en.m.wikipedia.org_wiki_Cat (2).png
Apr 23 2019, 2:27 PM
F28744602: en.m.wikipedia.org_wiki_Cat (1).png
Apr 23 2019, 2:27 PM
View All 12 Files
Subscribers
Aklapper
Edtadros
Jdlrobson
JTannerWMF
kostajh
• Niedzielski
ovasileva
pmiazga

Description

Steps to reproduce

  1. Trigger a notification on your account. For example, as an anonymous user leave a message on your own talk page.
  2. Login and visit https://en.m.wikipedia.beta.wmflabs.org/wiki/Barack_Obama on the Vector desktop site.
  3. Verify you have notifications.
  4. Switch to the MinervaNeue mobile skin.

Expected results

  • The MinervaNeue mobile skin notification count matches the Vector desktop skin.

Actual results

  • No notification alert is shown.

en.wikipedia.org_wiki_Dog.png (1×1 px, 2 MB)

en.m.wikipedia.org_wiki_Dog.png (1×1 px, 881 KB)

Environments observed

  • Browser version: Chromium v73.0.3683.86
  • OS version: Ubuntu v18.10
  • Device model: Desktop
  • Device language: English

Check any additional observations

Developer notes

Rendering the notifications overlay calls

markAllReadButton.toggle( false );

This should not happen - it should only happen if the mark all as read button is clicked.

See replication steps: T220979#5130638

QA Results

ACStatusDetails
1✅T220979#5340930
2✅T220979#5340930

Details

SubjectRepoBranchLines +/-
Do not automatically mark notifications on mobile as readmediawiki/extensions/MobileFrontendmaster+2 -5
Customize query in gerrit

Related Objects

Event Timeline

• Niedzielski created this task.Apr 15 2019, 2:00 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 15 2019, 2:00 PM
• Niedzielski mentioned this in T220511: [Bug] Wrong number of notifications shown on initial edit.Apr 15 2019, 2:05 PM
ovasileva triaged this task as Medium priority.Apr 16 2019, 10:25 AM
ovasileva moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.
ovasileva subscribed.

@Niedzielski - should this and T220511 be separate tasks?

• Niedzielski added a comment.Apr 16 2019, 2:04 PM

I think so, at least for now. T220511 shows a number but it's incorrect. The issue in this bug is that no alert is shown.

kostajh added a project: Notifications.Apr 16 2019, 2:05 PM
kostajh moved this task from Inbox to Needs Discussion on the Growth-Team board.
SBisson mentioned this in W2269 Echo Tasks.Apr 17 2019, 3:00 PM
Jdlrobson edited projects, added Web-Team-Backlog (Tracking); removed Web-Team-Backlog.Apr 23 2019, 1:42 AM
Jdlrobson subscribed.

Can somebody in growth look at this? It seems we source this number from Echo APIs so I'm guessing we are either accessing incorrectly or there is a bug in Echo... let me know if web can help in any way.

kostajh subscribed.Apr 23 2019, 1:56 PM

I can't reproduce this locally on HEAD of Minerva, MobileFrontend and Echo, or on beta. @Niedzielski are you only seeing this on beta? The job queue was broken there (T215339, fixed 4/19) and that may have impacted this.

• Niedzielski added a comment.Apr 23 2019, 2:27 PM

Here's what I see on prod enwiki:

  1. Login
  2. Visit https://en.wikipedia.org/wiki/Cat
    en.wikipedia.org_wiki_Cat.png (2×2 px, 1 MB)
  3. Visit https://en.m.wikipedia.org/wiki/Cat
    en.m.wikipedia.org_wiki_Cat.png (1×2 px, 1 MB)
  4. Visit https://en.wikipedia.org/wiki/User_talk:Niedzielski anonymously and add a new section
  5. Refresh https://en.wikipedia.org/wiki/Cat
    en.wikipedia.org_wiki_Cat (1).png (1×2 px, 1 MB)
  6. Refresh https://en.m.wikipedia.org/wiki/Cat
    en.m.wikipedia.org_wiki_Cat (1).png (1×2 px, 1 MB)
  7. Open and close the notification panel on mobile (don't click notifications)
    en.m.wikipedia.org_wiki_Cat (2).png (1×2 px, 1 MB)
  8. Refresh https://en.m.wikipedia.org/wiki/Cat; is this the wrong number?
    en.m.wikipedia.org_wiki_Cat (3).png (1×2 px, 1 MB)
  9. Refresh https://en.wikipedia.org/wiki/Cat
    en.wikipedia.org_wiki_Cat (2).png (1×2 px, 1 MB)

Step 8 seems like a bug to me.

kostajh added a comment.Apr 23 2019, 2:55 PM

Thanks @Niedzielski. Given the described behavior, this seems like a bug with Minerva or MF, not with the Echo API.

ovasileva edited projects, added Web-Team-Backlog; removed Web-Team-Backlog (Tracking).Apr 30 2019, 11:16 AM
JTannerWMF moved this task from Needs Discussion to External on the Growth-Team board.Apr 30 2019, 6:28 PM
Jdlrobson updated the task description. (Show Details)Jul 9 2019, 10:15 PM
gerritbot added a comment.Jul 9 2019, 10:15 PM

Change 521779 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Do not automatically mark notifications on mobile as read

https://gerrit.wikimedia.org/r/521779

gerritbot added a project: Patch-For-Review.Jul 9 2019, 10:15 PM
Jdlrobson moved this task from Needs Prioritization to Triaged but Future on the Web-Team-Backlog board.Jul 9 2019, 10:15 PM

@kostajh was correct. Luckily the solution was quite trivial - there's a line that needs removing.

gerritbot added a comment.Jul 11 2019, 6:24 PM

Change 521779 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Do not automatically mark notifications on mobile as read

https://gerrit.wikimedia.org/r/521779

ReleaseTaggerBot added a project: MW-1.34-notes (1.34.0-wmf.14; 2019-07-16).Jul 11 2019, 7:00 PM
Maintenance_bot removed a project: Patch-For-Review.Jul 11 2019, 7:10 PM
Jdlrobson edited projects, added Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1); removed Web-Team-Backlog.EditedJul 11 2019, 7:22 PM

A patch has been merged (worked on a related bug fix while working on the notification icon refresh). Per retro putting through QA (I'm not sure if Growth team should be QAing this or not cc @JTannerWMF .

Jdlrobson moved this task from Needs Analysis to Blocked on Others on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.Jul 11 2019, 7:22 PM
Jdlrobson moved this task from Blocked on Others to Needs QA on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.
Jdlrobson added a subscriber: JTannerWMF.
Edtadros claimed this task.Jul 15 2019, 1:54 PM
Edtadros added subscribers: pmiazga, Edtadros.

Per my 1:1 with @pmiazga I will QA this in production.

Edtadros reassigned this task from Edtadros to ovasileva.Jul 17 2019, 1:12 PM
Edtadros moved this task from Needs QA to Ready for Signoff on the Web-Team-Backlog (Readers-Web-Kanbanana-2019-20-Q1) board.

Test Result

Status: ✅ PASS
OS: macOS Mojave
Browser: Chrome
Device: iPhoneX

Test Artifact(s):

QA Steps

Trigger a notification on your account. For example, as an anonymous user leave a message on your own talk page.
Login and visit https://en.m.wikipedia.org/wiki/Barack_Obama on the Vector desktop site.

✅ AC1: Verify you have notifications.

IMG_3313.PNG (2×1 px, 806 KB)

✅ AC2: Switch to the MinervaNeue mobile skin and verify notifications are the same.

IMG_3311.PNG (2×1 px, 2 MB)

IMG_3312.PNG (2×1 px, 418 KB)

Edtadros updated the task description. (Show Details)Jul 17 2019, 1:13 PM
ovasileva closed this task as Resolved.Jul 17 2019, 2:24 PM

Looks good!

• Niedzielski mentioned this in T214540: [EPIC] AMC Navigation - user menu.Jul 25 2019, 6:45 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits