Page MenuHomePhabricator

mobile web media viewer shows icons
Closed, ResolvedPublic2 Estimated Story Points

Description

go to https://en.wikipedia.org/wiki/Bed or iOS app go to media viewer and click through to the last image...no icons
go to https://en.m.wikipedia.org/wiki/Bed go to media viewer and click through to the last image...ICONS

Acceptance criteria

  • If the class noviewer or metadata is added to an image or some parent node it should not be possible to scroll to it via the left and right buttons
  • When clicking such an image in the page it should not load the Mediaviewer

Development notes

  • Update page.getThumbnails to not return images with the noviewer/metadata class
  • Add a test for an image with the noviewer or metadata class
  • Add a test for an image which is inside an element with the noviewer or metadata class

e.g.

<div class="noviewer"><div><img></div></div>

Event Timeline

Jdlrobson subscribed.

Looking at this, I don't actually think the media viewer is at fault - the underlying template is.
If you scroll to the bottom of the page (External Links) it renders 3 templates.
If you click the icon inside these templates they go to the file page. Why are these icons links?

{{wikiquote|Beds}}
{{Wiktionary|bed}}
{{Commons category|Beds}}
Jdlrobson added a subscriber: kaldari.

According to @kaldari our logos are not public domain so we have to link to them.
It seems strange to me @JKatzWMF - worth talking to legal about making these public domain and not having to make the icons themselves clickable?

We can of course hack our way around this by filtering out the images from multimedia viewer for any images that have width or height < 50px in the article.

(MultimediaViewer desktop and mobile are different)

The mobile media viewer should ignore images in/with noviwer/metadata classes like the desktop one does.

Jdlrobson triaged this task as Medium priority.May 1 2017, 5:40 PM
Jdlrobson updated the task description. (Show Details)

Changed the criteria a bit to match desktop behavior.

Why are both needed @Tgr ?
Can you give me an example of an image which has metadata but not noviewer ?

Various templates like {{ambox}} use it. (That one gets stripped by MobileFrontend anyway, but others might not.) metadata is more generic (it will hide the contents in the print view as well, for example). noviewer is only used when something needs to be disabled in MediaViewer specifically (e.g. CSS-based map hacks). That's the theory anyway, I don't think they get used with much consistency.

Also, desktop MediaViewer ignores images which link to somewhere else than the file page; you might or might not want to copy that. It is often useful but also has some very annoying failure modes such as T75902.

(There are docs for this, but hard to find.)

Jdlrobson set the point value for this task to 2.May 30 2017, 4:38 PM

FYI @JMinor and @Dbrant you may have the same issue in your apps - you may want to create a task that does the same as this will only fix mobile web.

Change 360566 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Exclude noviewer and metadata images from media viewer

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

Change 360566 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Exclude noviewer and metadata images from media viewer

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

Tried this on a variety of browsers and platforms. The icons are appearing.

The icons are appearing.

The icons are appearing or NOT appearing?
(Note this can only be tested on the beta cluster)

I can still see the icons on beta cluster

Sorry, I must have misread or misunderstood. I assumed that on mobile we had more of a carousel mediaviewer experience. Is the desired behavior to 'stop' showing/allowing scroll at the ends of the slideshow?

@phuedx @ABorbaWMF I believe Nirzar is referring to this icon showing up in the media viewer you have not misread or misunderstood:

Screen Shot 2017-06-27 at 9.03.19 AM.png (500×552 px, 128 KB)

The desired behaviour is to allow scrolling at the ends of the slideshow, that's no the issue here.

Reminder: Pictures paint a thousand words!

Moving back to "to do" as we have a new test case:

<table class="plainlinks metadata ambox ambox-content ambox-Unreferenced" role="presentation"><tr><td class="mbox-image">
<div style="width:52px"><a href="/wiki/File:Question_book-new.svg" class="image"><noscript><img alt="" src="https://upload.wikimedia.org/wikipedia/commons/thumb/9/99/Question_book-new.svg/50px-Question_book-new.svg.png" width="50" height="39" data-file-width="262" data-file-height="204"></noscript><span class="lazy-image-placeholder" style="width: 50px;height: 39px;" data-src="https://upload.wikimedia.org/wikipedia/commons/thumb/9/99/Question_book-new.svg/50px-Question_book-new.svg.png" data-alt="" data-width="50" data-height="39" data-srcset="https://upload.wikimedia.org/wikipedia/commons/thumb/9/99/Question_book-new.svg/75px-Question_book-new.svg.png 1.5x, https://upload.wikimedia.org/wikipedia/commons/thumb/9/99/Question_book-new.svg/100px-Question_book-new.svg.png 2x"> </span></a></div>
</td>
</tr>
</table>
Jdlrobson added a subscriber: ABorbaWMF.

Change 361721 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/MobileFrontend@master] Check parents when parsing thumbnails

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

Change 361721 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Check parents when parsing thumbnails

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

Over to you, @ABorbaWMF! As @Jdlrobson explained in T163847#3382911, you're looking for no regressions in the mobile media viewer as well as images like https://en.m.wikipedia.beta.wmflabs.org/wiki/Bed#/media/File%3AQuestion_book-new.svg not showing up.

Ok, tried it on a variety of mobile devices/browsers as well as desktop OSs/browsers. No placeholder images appear on the test article.

https://en.wikipedia.beta.wmflabs.org/wiki/Bed