Page MenuHomePhabricator

UI rendering issue when no notifications
Closed, ResolvedPublic

Description

Visit Special:Notifications on desktop when you have no recent activity.
Observe the misplacement of the 0

Details

Related Gerrit Patches:
mediawiki/extensions/Echo : masterFallback to dbname if source title doesn't exist

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 28 2017, 7:36 PM
Catrope added a subscriber: Catrope.May 2 2017, 5:55 PM

The name of the current wiki should appear in that box, it shouldn't be empty

I don't have this on my computer, @Jdlrobson. What is your browser?

Jdlrobson added a comment.EditedMay 2 2017, 6:11 PM

The name of the current wiki should appear in that box, it shouldn't be empty

How is that configured? I'm seeing this on my local instance on all browsers so maybe this is a problem only for 3rd parties. If it is a configuration issue, it would be useful to provide a default or at least a &nbsp.

The issue is present on http://cx2.wmflabs.org/index.php/Special:Notifications - is this actually a bug that needs to be fixed?

Mooeypoo added a subscriber: Mooeypoo.EditedMay 29 2017, 10:14 AM

The local wiki name comes from the API request for 'unreadPagesByWiki' (the controller call is mw.echo.Controller.prototype.fetchUnreadPagesByWiki and the actual API module is query with meta unreadnotificationpages) which is also where the system attaches the name of the local wiki to the source.

Could it be that there's an issue with that call? @Jdlrobson since I can't reproduce this, can you check your network tab for the response for this request? I suspect we have an issue there, perhaps something that needs to be enabled.

In any case, I'm going to add a backup fallback that will have the literal string "local" or something like that, in case no name is found at all, so that we don't get the terrible presentation we see above. But this shouldn't really happen; if you only have one source, the API call should return that single source anyways.

@Catrope could it be that if the wiki doesn't have cross-wiki capability or multiple wikis, etc we don't get the response we expect? Again, I can't reproduce this and I can't access @Etonkovidova's link to check, but I am suspecting that since it seems to happen in a separate wiki and a local wiki, it might be an issue with something about the API call not returning "local" info when it's the only wiki available. Could that be?

I'm testing this in a mediawiki wiki (not vagrant) which is not setup with cross wiki capabilities which is probably a typical 3rd party setup.


Happy to show you this in person this afternoon if you want to come to my desk!

Mooeypoo added a comment.EditedMay 30 2017, 5:25 PM

I'd love to see it but it would have to wait until I get back to SF :)

It's super odd. Can you check the response if the API call to unreadnotificationpages?

http://localhost:8888/w/api.php?action=query&format=json&meta=unreadnotificationpages&uselang=en&unpgrouppages=true&unpwikis=*&_=1496188730679
returns

Note unpwikis warning:

{"warnings":{"main":{"*":"Unrecognized parameter: unpwikis."}},"batchcomplete":"","query":{"unreadnotificationpages":{"my_wiki":{"pages":[{"ns":2,"title":"User:Jdlrobson","unprefixed":"Jdlrobson","pages":["User:Jdlrobson","User talk:Jdlrobson",null],"count":1}],"totalCount":1,"source":{"title":null,"url":"http://localhost:8888/api.php","base":""}}}}}

unpwikis being unrecognized might mean that cross-wiki notifications are disabled?

@Mooeypoo the CX link in my previous comment is now: http://cx-testing.wmflabs.org/index.php/Special:Notifications (they changed it recently). There is no crossiwki notificaitons enabled for that site. Btw, you'll need a different user account specifically for that site.

I still do not see the issue happening for wikis where cross-wiki notifications are enabled.

Ah ha!

http://localhost:8888/w/api.php?action=query&format=json&meta=unreadnotificationpages&uselang=en&unpgrouppages=true&unpwikis=*&_=1496188730679
returns
Note unpwikis warning:

{"warnings":{"main":{"*":"Unrecognized parameter: unpwikis."}},"batchcomplete":"","query":{"unreadnotificationpages":{"my_wiki":{"pages":[{"ns":2,"title":"User:Jdlrobson","unprefixed":"Jdlrobson","pages":["User:Jdlrobson","User talk:Jdlrobson",null],"count":1}],"totalCount":1,"source":{"title":null,"url":"http://localhost:8888/api.php","base":""}}}}}

Note in the API reply for my_wiki which I assume is the local DBName, below, in the details we receive, it has title: "null"

So first, @Catrope do you know how this happened in the backend? I thought title for local wiki can't be null...?

But regardless of fixing that, we should make sure the pages model uses the title if it exists and falls back on DBName if it doesn't, when it defines the sources on initialization from the controller. I thought it already did that, but it may not. I don't currently have access to the code but I'll look into it tomorrow.

Right, it uses $wgConf magic, and if that's not set up correctly on your wiki then it won't work very well. Maybe MW-Vagrant doesn't configure this correctly if you don't enable the CentralAuth rule? But I thought Echo pulled in CentralAuth automatically?

Right, it uses $wgConf magic, and if that's not set up correctly on your wiki then it won't work very well. Maybe MW-Vagrant doesn't configure this correctly if you don't enable the CentralAuth rule? But I thought Echo pulled in CentralAuth automatically?

I might be wrong, but I think @Jdlrobson said his installation is non-vagrant?

In any case, I'm going to add a safety-check; if there's no name from the API, fall-back on dbname.

Change 357205 had a related patch set uploaded (by Mooeypoo; owner: Mooeypoo):
[mediawiki/extensions/Echo@master] Fallback to dbname if source title doesn't exist

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

I used a slightly convoluted way to test this, so I can't say I managed to properly test it out. @Jdlrobson when this is merged, can you retest? (Or if you can pull and test, even better!)

This should at least make sure that there's no empty label - it will fall back on the local wiki's name or the label 'local'.

Change 357205 merged by jenkins-bot:
[mediawiki/extensions/Echo@master] Fallback to dbname if source title doesn't exist

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

The feedback from @Jdlrobson is, definitely, needed.
My vagrant always showed db names:


I tried to disable/comment out $wgEchoCrossWikiNotifications (and $wgEchoUseCrossWikiBetaFeature) since @Jdlrobson mentioned that the wiki where the issue was observed was "not setup with cross wiki capabilities". Db name is always displayed.

The fix did not correct not-displayed wiki names on http://cx-testing.wmflabs.org/index.php/Special:Notifications (due to totally different setup?). Btw, not displayed wikinames have nothing to do with whether there are no unread notifications or there are unread notifications.

is how it looks on my local computer now which is fixed from my perspective.
http://cx-testing.wmflabs.org/ code needs updating. If you check Special:Version it hasn't got the latest patch.

Mooeypoo added a comment.EditedJun 7 2017, 5:30 PM

Just a note here, this is solving the symptom, but not the deeper issue - can we make sure all wikis always have a name in the wg variable, or are we missing a documentation piece somewhere that we can fix? Ping @Catrope @Jdlrobson @Mattflaschen-WMF

Mattflaschen-WMF added a comment.EditedJun 7 2017, 6:05 PM

Just a note here, this is solving the symptom, but not the deeper issue - can we make sure all wikis always have a name in the wg variable, or are we missing a documentation piece somewhere that we can fix? Ping @Catrope @Jdlrobson @Mattflaschen-WMF

We should probably:

  • Assume if cross-wiki notifications are enabled, wgConf is set up.
  • If it's not (as here)
    • Omit 'unpwikis' from the API request.
    • Instead use wgSitename, which is always available (on the client-side already too).

We can also omit the sidebar completely if crosswiki isn't set up. Just beware of mobile, but then it will only have the filters.

jmatazzoni closed this task as Resolved.Jun 15 2017, 11:19 PM
jmatazzoni claimed this task.