Page MenuHomePhabricator

Link to original file is not a link and can not be used as such
Closed, ResolvedPublic

Description

Steps to reproduce:

  1. Open an image in the viewer.
  2. Click the middle mouse button on "View original file" to open it in a new tab.

Nothing happens.

Or right click and try to open the link in a new tab. The function is not there because it's not a link. Why is that? I'm a developer and I really can't think of a reason to not make this a link.

Marking this as major because it breaks users expectations and UX flow and is one of many reasons why experienced users dislike the feature.


Version: master
Severity: major

Event Timeline

bzimport raised the priority of this task from to High.Nov 22 2014, 3:43 AM
bzimport added a project: MediaViewer.
bzimport set Reference to bz69469.
bzimport added a subscriber: Unknown Object (MLST).

Thanks for the report, Thiemo. It looks easy enough to fix if you don't mind losing the click tracking. If you wanted to keep the click tracking, you would need to have the links go via a redirector, like external links in Google, Facebook, etc.

(In reply to Tim Starling from comment #1)

if you don't mind losing the click tracking.

You can have the exact same click handler on an <a> element. The <a href="..."> is for right click, middle click and such, the event handler does the same as it does now. You will not loose anything.

We could click right click separately and then 'assume' that those would be the 'copy link' actions. Seems possible to me. I'm a fan of using <button> and <a> where ever we should.

(In reply to Thiemo Mättig from comment #2)

You can have the exact same click handler on an <a> element. The <a
href="..."> is for right click, middle click and such, the event handler
does the same as it does now. You will not loose anything.

Navigating away from a page causes XHR requests to be aborted. So if the user left-clicks on the link, and you send a clicktrack request to the server and return true from the event handler, the clicktrack request will be immediately aborted when the browser navigates to the href target. The current behaviour is to wait for the clicktrack request to complete and then to change the location in the current window.

Note that we only track 1 in every 10,000 clicks at the moment. I would question whether the data is valuable enough to justify the drawbacks of either clicktracking method.

  • Bug 68203 has been marked as a duplicate of this bug. ***

There is no difference in click tracking between an <a> and a <span> with a click handler. Either way you can choose to delay navigation (which degrades user experience) or risk loosing the log event. (This is bug 42815, by the way. See also bug 44480.)

The problem is that the thumbnail might be available sooner than the link to the full image; in such a case, we delay the handling of the click until the imageinfo API request returns. There is no way to delay a middle/right click. As mentioned in bug 68203, the alternative would be to not show the icon until the data is present; that looks somewhat ugly but on the whole probably the better solution.

We could also try to guess the URL of the full image from the thumbnail URL, instead of relying on the API; we already have code for that, but so far we only used it in situations when a 404 can be caught and recovered from (i.e. to fetch the thumbnails displayed by MediaViewer). In theory, reconstructing the original file URL is really simple; still, it's a bit scary to do that when we have no way to detect that we might be sending the user to an error page.

It would be possible to reconstruct the source URL on the client side, but please be careful of abbrvThreshold, which is used on WMF. The source filename is only available in the thumbnail directory, not in the part of the thumbnail filename before the hyphen.

You should respect the repo's zone URL rather than assuming source files and thumbnails will be on the same server.

If thumbScriptUrl is used, it is somewhat more complicated, since an MD5 hash would have to be done on the client side. This is used on private WMF wikis, such as office.wikimedia.org.

An alternative would be to have the parser serialize the image data and embed it in the page for MMV to use. The parser loads all image information already, so having MMV load it as well is a bit redundant.

The code to guess an image URL from another one is at https://github.com/wikimedia/mediawiki-extensions-MultimediaViewer/blob/master/resources/mmv/provider/mmv.provider.GuessedThumbnailInfo.js#L259

It is not affected by abbreviated file names (we have a unit test for that). It does not respect zone URLs since those are not available without an API request, and the goal of URL guessing is to be able to load the image immediately. We could load the repo information via ResourceLoader, since it basically never changes; still, that's a minor issue, given that WMF servers are not affected, and on other servers URL guessing is disabled by default due to various other problems (image_auth.php, reverse proxies, lack of a 404 handler). I have opened bug 69558 for that.

(In reply to Tim Starling from comment #7)

An alternative would be to have the parser serialize the image data and
embed it in the page for MMV to use. The parser loads all image information
already, so having MMV load it as well is a bit redundant.

We already do this for image dimensions because not having those really degrades user experience. bug 67302 comment #4 suggests that's not a future-compatible thing to do and should be avoided whenever possible.

Proposal:

  • change the original file button to a proper link
  • change the original file button to only appear when the URL becomes available
  • when $wgUseThumbnailGuessing is true, use thumbnail guessing to ensure the URL is available immediately. That should be safe on WMF servers, and should be safe everywhere once bug 69558 is fixed.

(In reply to Tisza Gergő from comment #8)

(In reply to Tim Starling from comment #7)

An alternative would be to have the parser serialize the image data and
embed it in the page for MMV to use. The parser loads all image information
already, so having MMV load it as well is a bit redundant.

We already do this for image dimensions because not having those really
degrades user experience. bug 67302 comment #4 suggests that's not a
future-compatible thing to do and should be avoided whenever possible.

I wouldn't recommend putting metadata in the content HTML, I would put it in JavaScript, like what I did in Scribunto to display error backtraces.

Gilles lowered the priority of this task from High to Medium.Nov 24 2014, 2:19 PM
Gilles added a subscriber: Gilles.

Mass-removing the Multimedia tag from MediaViewer tasks, as this is now being worked on by the Reading department, not Editing's Multimedia team.

MBinder_WMF added a subscriber: MBinder_WMF.

As this task is 2 years old, and hasn't been touched since September 2014, it will be closed. Please (politely) re-open the task if it remains an issue. :)

Change 303142 had a related patch set uploaded (by Thiemo Mättig (WMDE)):
Make the download/share buttons actual links for right/middle click

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

Should review and improve patch fairly soon.

Seems this dropped off the radar again ?

The former div has been changed to a button in T58471. Can't test, if problem remains with a button element due to missing mouse with middle click functionality.

Change 303142 merged by jenkins-bot:
[mediawiki/extensions/MultimediaViewer@master] Make the download/share buttons actual links for right/middle click

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