Page MenuHomePhabricator

Thumbnails in popups don't appear in Opera 12
Closed, ResolvedPublic

Description

Thumbnails in popups don't appear in Opera 12.

Event Timeline

matmarex created this task.Mar 30 2017, 2:02 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 30 2017, 2:02 PM

Change 345594 had a related patch set uploaded (by Bartosz Dziewoński):
[mediawiki/extensions/Popups@master] Properly create the <svg> and <image> elements for thumbnails

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

ovasileva triaged this task as High priority.Mar 30 2017, 10:02 PM
ovasileva added subscribers: ABorbaWMF, ovasileva.

@ABorbaWMF - could you look at this one? I'm not sure if we were already testing Opera already

@ovasileva I did some testing on Opera browsers, but concentrated on the more recent versions. I did a quick sampling of Opera versions on various Windows systems.

Opera versions 43 40 37 35 32 30 26 24 - Everything appears to fine

Opera versions 23 21 19 15 - Displays image alignment problem on some images


But not on others

Opera versions 12.12 12 - No image appears

Note that Opera 15 and later use the Blink rendering engine, same as Chrome etc. – so if you test on Chrome, testing on modern Opera is not really necessary. Opera up to version 12 used the Presto rendering engine (Opera 15 was effectively a total rewrite; they skipped 13 and 14). Many of its features haven't been re-implemented in the new one, so some folks still use the old browser.

I'm happy to help with Opera 12 support, mainly for nostalgia reasons ;) In my experience it does not require much effort, and often Opera 12 bugs help uncover other issues in the code (like in this task, where I fixed it by replacing some ugly hacks).

Change 345594 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Properly create the <svg> and <image> elements for thumbnails

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

phuedx added a subscriber: phuedx.EditedApr 3 2017, 11:26 AM

@ABorbaWMF: Could you rerun your tests on the Beta Cluster with a number of browsers, including Opera 12?

Given that the latest Opera version is 44, and Opera 12 was released about 6 years ago, I wonder if we should drop support for Opera 12, which is not in the list of our Grade A browsers. @phuedx do you mind if we close this task as wontfix?

The version numbers don't really matter, since they changed the versioning scheme after 12. The latest update for Opera 12, Opera 12.18, was actually released in February 2016. And Opera 12.1+ is actually listed as supported at Grade A on the page you link, under "Browser support matrix".

The patch was in fact already merged, and the fix involved removing 30 lines of code and adding 6. This task is already fixed, I hope you don't want to revert it out of principle.

Given that PagePreviews work perfectly on Opera 12 now, it seems silly to drop support for it (I assume this means blacklisting it somehow). It would be reasonable to simply not officially support it, but then that is already what you do (I don't think anyone ever claimed that it's supported).

@matmarex thanks for clarifying. If it's already fixed, then I have no objection. I should have noticed that version 12 has recently been updated.

@ABorbaWMF, would you mind testing on Opera 12.18, which's been released about a year ago.

I didn't have opera 12.18 crossbrowsertesting (the emulator), but I do have Opera 12.14 on Mac OSX. I ran a quick test with Opera 12.14 on this page: https://en.wikipedia.beta.wmflabs.org/wiki/Dog

Page previews are appearing normally.

bmansurov moved this task from Needs Analysis to Ready for Signoff on the Reading-Web-Sprint-95 board.
bmansurov reassigned this task from matmarex to ovasileva.

Using crossbrowsertesting I checked three versions of Opera 12 using the Mac OSX 10.11 setting, and according to the screencaps below page previews appear as expected.

Mac OSX 10.11 Opera 12.14

Mac OSX 10.11 Opera 12.12

Mac OSX 10.11 Opera 12.00

phuedx added a comment.Apr 6 2017, 8:12 AM

The patch was in fact already merged, and the fix involved removing 30 lines of code and adding 6.

30 lines of hacks that were introduced to work around the use of an incorrect namespace! Thanks, @matmarex.

ovasileva closed this task as Resolved.Apr 6 2017, 11:11 AM

looks good, thanks all