Page MenuHomePhabricator

Display banners on diff pages
Open, Stalled, NormalPublic

Description

When viewing a diff page the page banner doesn't render anywhere in the display. See for example:
https://it.wikivoyage.org/w/index.php?title=Utente:Pietro_Di_Fontana&curid=41359&diff=489186&oldid=487345

Notice how the banner does not appear in the preview on the diff but appears when you view the page:


As @Andyrom75 later writes:

As we know diff pages are used to have a preview of the details changed in a page and the saved final result. This is helpful also to patroller to understand if the changes are fine or not.
Currently, the extension do not show the banner in diff pages, and this bug provoke two issues:

  1. the shown page is aesthetically different from the saved one, so for example I cannot use a diff page to show which was the graphic effect of an old banner
  2. when an user change the banner of a page (like the example above), a patroller cannot validate immediately the action, because he's not able to see if the new file name exist (in this case no banner is shown) or if existing if it's appropriate (maybe has been linked a wrong banner in terms of dimension or content) or last but not the least if it's better than the previous one. In this cases the patroller must open the last saved version to see the banner, than he has to change manually the file name and go in preview mode to see the old one, because the diff do not show the old banner.

Previously diffs were disabled intentionally as part of T110384. Sounds like we want to revisit this discussion.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptOct 12 2016, 5:49 PM
Restricted Application added a project: Wikidata. · View Herald TranscriptOct 13 2016, 10:20 AM
Jdlrobson renamed this task from Extension:WikidataPageBanner do not show banner in diff pages to Do not show site notice / fundraising banners in diff pages.Oct 13 2016, 5:16 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson added a subscriber: Jdlrobson.

@Andyrom75 please correct the description and re-add the Wikidata-Page-Banner tag if I've not represented the problem correctly but I do not see a Wikidata page banner at the top of the page.

Jdlrobson renamed this task from Do not show site notice / fundraising banners in diff pages to Problem with display of Wikidata page banners on diff pages.Oct 13 2016, 5:22 PM

Sorry.. I can see different ways to interpret this problem but now I re-read your initial report I suspect that you were referring to the wikidata page banners.

I'm still not sure I understand the problem.
Could you articulate what you expect to happen and what you are seeing?

I don't see banners on any diffs which I believe was the requested behaviour when we built it.

@Jdlrobson I'll try to explain better the issue.

As we know diff pages are used to have a preview of the details changed in a page and the saved final result. This is helpful also to patroller to understand if the changes are fine or not.

Currently, the extension do not show the banner in diff pages, and this bug provoke two issues:

  1. the shown page is aesthetically different from the saved one, so for example I cannot use a diff page to show which was the graphic effect of an old banner
  2. when an user change the banner of a page (like the example above), a patroller cannot validate immediately the action, because he's not able to see if the new file name exist (in this case no banner is shown) or if existing if it's appropriate (maybe has been linked a wrong banner in terms of dimension or content) or last but not the least if it's better than the previous one. In this cases the patroller must open the last saved version to see the banner, than he has to change manually the file name and go in preview mode to see the old one, because the diff do not show the old banner.

The old banner system works correctly. If this behaviour has been requested by someone, that someone hasn't considered the above implications. I'm sorry for that.

By the way I suppose that it shouldn't take so long to fix this bug.

Let me know if this time I was able to explain you better the situation or not.

Thanks in advance for your support

Jdlrobson renamed this task from Problem with display of Wikidata page banners on diff pages to Display banners on diff pages.Oct 17 2016, 4:59 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson updated the task description. (Show Details)

@Andyrom75 I looked into this and it was turned off intentionally but annoying I didn't link to any Wikivoyage community discussion. See T110384 for a little background. @Sumit do you remember why we decided to do this? @Nicolas_Raoul was there any community input around this? Can we revisit this?

Andyrom75 added a subscriber: Tgr.Oct 17 2016, 6:54 PM

@Jdlrobson thanks for sharing the available background.
As briefly wirtten in T110384 by @Tgr and @Nicolas_Raoul banner should stay there.
If there is someone, or maybe a whole language community that for any reason would prefer to see/provide a diff page different from the the real one should be able to do it through a dedicated gadget parameter or extension configuration, but IMHO hide the banner for all the user is not the right approach.

I agree and I'm regretful that I wasn't more verbose in that task. If I created it, I suspect there was a good reason - e.g. a discussion on Wikivoyage or there was a technical barrier to doing so. Hoping we can shed some light on the issue and get it fixed. Thanks for reporting it!

ovasileva triaged this task as Normal priority.Oct 20 2016, 7:11 PM
Jdlrobson updated the task description. (Show Details)Oct 20 2016, 7:14 PM

Hi @Jdlrobson, do you think that more verifications are needed or is already possible to restore the normal visualization of the diff pages?

@Andyrom75 I think that's fine. It may be tricky to fix however as 1) I need to find someone to work on this problem or review a patch from me and 2) the title of the page does not currently appear in the diff.

Sumit added a comment.Nov 3 2016, 6:03 AM

I agree and I'm regretful that I wasn't more verbose in that task. If I created it, I suspect there was a good reason - e.g. a discussion on Wikivoyage or there was a technical barrier to doing so. Hoping we can shed some light on the issue and get it fixed. Thanks for reporting it!

@Jdlrobson is there a way to look up past Traveller's pub discussions? I guess we did this because the diff for banners was not making sense( there would be only one banner image ), but I'm not so sure.

@Andyrom75 I think that's fine. It may be tricky to fix however as 1) I need to find someone to work on this problem or review a patch from me and 2) the title of the page does not currently appear in the diff.

Regarding 2), T110384 says:

On reflection given that the page headings do not render in revisions and the banner is substitution for the heading I actually think this is the correct behaviour. For instance the heading "Download" does not show up in https://www.mediawiki.org/w/index.php?title=Download&type=revision&diff=1896349&oldid=1813316

I'll submit a patch to this once I get back home ( after 6th ).

@Sumit all conversations are archived but we'd have to search and find the conversation. I think the issue will be the fact that usually the banner takes over the title of the page and in this case it would not - it would render under the diff. It might be tricky to implement. Another possible concern is that the banner takes up a lot of space and for most diff previews will get in the way of the rest of the preview.

Will watch out for a patch from you :)
Also it would be great to recruit some other volunteers to work on this extension. There is a bit of a backlog growing and it would be good to have some more maintainers!

Andyrom75 added a comment.EditedNov 14 2016, 11:56 AM

@Sumit, @Jdlrobson I don't know if the patch has been already submitted/reviewed or not.

Anyway I take the chance to highlight another minor issue with the preview, but honestly I do not recall the previous behaviour.
If I preview the whole page, the banner is correctly shown, but appears also the standard wikiindex that do not allow to see correctly the final layout.
Let me know if you think that I should open a new separate phabricator request or not.

Thanks

@Andyrom75 no progress yet - I am a bit short on volunteers. I'll send a mail to the mailing list to see if I can get some engineers interesting in helping me maintain this extension.

Could you please create a task for that other issue. I'm not sure I completely understand the issue, so the more detail you can give in a new task (e.g. url it happens on, browser, screenshots) the better!

Change 321369 had a related patch set uploaded (by Sumit):
WPB enable banners on diff pages

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

@Sumit's patch puts the banner ABOVE the diff.

I worry this might be distracting to those who just want to view the diff.


It also doesn't show the title/display title

@Nicolas_Raoul @Andyrom75 how would you expect this to behave (I think this was why we disabled it before)?

Personally I don't see banner is not a big issue on diff pages.
If we have them, the beginning of the "Revision" section would probably be the most sensible place, though.

Is this worth a broader conversation on the wiki before implementation?

Putting the banner above the diff without the titla has to be considered a temporary patch that improve the current situation, but the real/final patch has to show banner (title included) below the diff.

At the end of the day, banner it's just an image, so it has to be shown where it is supposed to be, like al the other images.

PS @Jdlrobson, ok, for the other issue I'll open another task.

We could just show the banner (at the top of of the page) when the banner has changed..

Ping @Nirzar any thoughts on this discussion?

[Seems more complicated then we originally hoped]

Jdlrobson changed the task status from Open to Stalled.Nov 30 2016, 5:18 PM

We're not clear on the path forward right now :/

Maybe I'm wrong but since the banner it's just a normal picturre (nothing more, nothing less), I suppose that has been use some code to hide it, so it should be enough to remove/bypass that code.

It's trivial to show the banner again but I'm concerned that showing it at the top of the diff page will make it harder for patrollers to patrol given it will hide the rest of the diff on a small resolution. Given the opposite opinion it would be extremely helpful if this could be brought up on the travellers pub so we can work out if this is really an issue. I keep meaning to do this but haven't had time.

I agree that the very top of the page it's not the right place for the banner. The right place is its real position: at the top of the article (below the diff) and not at the top of the page.
As explained before, is an issue not seeing the banner and it must be corrected.
By the way I understand the lack of time... is a common issue :-)

The diff is shown where-ever the title is (which usually happens to be the top of the page and the top of the article)

However, on this diff page
"Differenze tra le versioni di "Utente:Pietro Di Fontana" is considered the title of the page and the actual title of the page is not present... hence turning on the banners will show it at the top of the page!

The old banner hid the page title and created a dedicated new title for the banner, maybe that's way in the old implementation this problem has never occur.
Is it possible to implement the banner in the same way?