Page MenuHomePhabricator

[Bug] Control clicking image doesn't open in new tab
Closed, ResolvedPublic

Description

Steps to Reproduce

  1. https://en.m.wikipedia.org/ on a DESKTOP browser.
  2. Control-click an image

Expected Results

  • Image opens in new tab just as it does for middle-clicking

Actual Results

  • Image opens in the current tab and destroys browsing context

Environments Observed

  • Production enwiki on Minerva

Browser Version

  • Chromium 69.0.3497.81

OS Version

  • Ubuntu v18.04

Device Model

  • Desktop

Device Language

  • English

Developer notes

Follow the logic of the MultimediaViewer extension: https://github.com/wikimedia/mediawiki-extensions-MultimediaViewer/blob/master/resources/mmv.head/mmv.head.js#L38

Do not interfere with non-left clicks or if modifier keys are pressed.

QA steps

This can be tested on beta cluster or reading web staging (dev should confirm this and delete this parenthesises when this is done)

Things to double check
On mobile:

  • Does clicking the image load the media overlay on mobile
  • Can I use open image in new tab (hold press) without opening the image overlay

On desktop:

  • Does clicking an image show the media overlay?
  • Can I open an image in a new tab without opening the image overlay

Event Timeline

Restricted Application changed the subtype of this task from "Deadline" to "Task". · View Herald TranscriptOct 5 2018, 5:47 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Subhash_17 added a subscriber: Subhash_17.
Subhash_17 removed Subhash_17 as the assignee of this task.Nov 4 2018, 10:49 AM
Subhash_17 removed a subscriber: Subhash_17.

I did a little bit of code digging:

If we're on the desktop the image click is handled by the MultimediaViewer extension in mmv.boostrap.js:430,
but on the mobile phone the click is handled by the MinervaNeue skin in init.js:26 and the extension gets notified by an hash change in mmv.js:903.

To change this behavior I would recommend to change the click handler in the MinervaNeue skin to open the picture correctly.
I hope this change doesn't break other extensions.

Change 474770 had a related patch set uploaded (by LukBukkit; owner: LukBukkit):
[mediawiki/skins/MinervaNeue@master] Don't open images if control keys are pressed

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

Jdlrobson removed LukBukkit as the assignee of this task.Nov 20 2018, 9:34 PM
Jdlrobson updated the task description. (Show Details)

Change 474770 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Don't open images if control keys are pressed

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

Seems to be working on Beta for both desktop and mobile

ovasileva closed this task as Resolved.Nov 27 2018, 6:41 PM
ovasileva added a subscriber: ovasileva.

looks good, thanks all!