Page MenuHomePhabricator

[Bug] Notification counts broken on mobile
Closed, ResolvedPublic

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.


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
1T220979#5340930
2T220979#5340930

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 15 2019, 2:00 PM
ovasileva triaged this task as Normal priority.Apr 16 2019, 10:25 AM
ovasileva moved this task from Incoming to Needs Prioritization on the Readers-Web-Backlog board.
ovasileva added a subscriber: ovasileva.

@Niedzielski - should this and T220511 be separate tasks?

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 moved this task from Inbox to Needs Discussion/Analysis on the Growth-Team board.
Jdlrobson added a subscriber: Jdlrobson.

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.

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.

Here's what I see on prod enwiki:

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

Step 8 seems like a bug to me.

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

Jdlrobson updated the task description. (Show Details)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

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

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

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 .

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

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.

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

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!