Wordmark not supported in IE8
Closed, ResolvedPublic

Description

The new wordmark in the mobile header isn't supported in IE8 because SVG without fallback.

To solve this we propose adding an onerror attribute to the printed logo that attempts to load a PNG when the SVG fails.
onerror="this.src=this.src.replace( '.svg', '.png' ); this.onerror=null;" />

For https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.svg there will also be a https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.png

Google code-in warning

This task is a little more trickier than other tasks that may be available but hopefully rewarding as you'll be adding logos to all Wikimedia projects on older browsers. Please bear this in mind when tackling this. @Jdlrobson will mentor.

Acceptance criteria

Volker_E created this task.May 31 2017, 2:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMay 31 2017, 2:49 PM

This is a known problem. IE8 doesn't account for a significant portion of mobile traffic. Do we want an IE8+Android specific fix for this?

Jdlrobson triaged this task as Low priority.May 31 2017, 4:54 PM
Jdlrobson moved this task from Backlog to New adventures on the MinervaNeue board.
Jdlrobson moved this task from New adventures to Bugs on the MinervaNeue board.

So if we want to do this i suggest doing the following:

onerror="this.src=this.src.replace( '.svg', '.png' ); this.onerror=null;" />

For https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.svg there will also be a https://en.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.png

Seems like a lightweight way to fix this.

@Volker_E what do you think? If so I'll prep it for Google-Code-in.

Jdlrobson moved this task from Inbox to Next up on the User-Jdlrobson board.Oct 31 2017, 12:28 AM

@Jdlrobson Do we have to do this with an inline onerror attribute? Apart from that sure, Google Code-in sounds fine to me.

We don't run is in ie8 so in my opinion it would be the cleanest solution. What would you suggest instead?

Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Next up to Tracking on the User-Jdlrobson board.Nov 7 2017, 8:06 PM

I will work on this today, but one question left: What size should be those converted png images?

Personally I would suggest a max. size of 128px, as it should be enough for those browsers and we are probably not dealing with high resolution displays.

divadsn claimed this task.Dec 2 2017, 11:02 AM

Change 394820 had a related patch set uploaded (by Divadsn; owner: Divadsn):
[operations/mediawiki-config@master] Add converted copyright svg images as png files

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

I can't find any clues where to add the failover for the logo, which repo am I supposed to look out for the MinervaNeue skin? :)

I can't find any clues where to add the failover for the logo, which repo am I supposed to look out for the MinervaNeue skin? :)

https://gerrit.wikimedia.org/r/#/admin/projects/mediawiki/skins/MinervaNeue

bd808 added a comment.EditedDec 3 2017, 4:07 AM

@divadsn It looks to me like the SkinMinerva::getSitename function is where the <img> tag that needs to be modified to add the onerror handler is generated.

Following how this all works is a bit confusing.

  • MinervaTemplate::render() builds up a $templateData array that is passed to $templateParser->processTemplate( 'minerva', $templateData );.
  • The 'minerva' there tells the template parser to load the minerva.mustache template file and then put the collection of $templateData into it.
  • The {{{headinghtml}}} mustache tag inside the <div class="branding-box"> element found in the template file eventually contains the <img src="/static/images/mobile/copyright/wikipedia-wordmark-en.svg" alt="Wikipedia" height="18" width="116"> tag.
  • You have to work backwards in MinervaTemplate::render() to find out where $templateData['headinghtml'] comes from. You can see in the source that it is populated with $data['footer-site-heading-html'].
  • Once I found this I "cheated" and searched the codebase for 'footer-site-heading-html' and found that SkinMinerva::prepareHeaderAndFooter() calls $tpl->set( 'footer-site-heading-html', $this->getSitename() );.
  • SkinMinerva::getSitename() confusingly has a doc comment saying that it "Returns the site name for the footer", but it turns out that it is used for both the header and the footer these days.
divadsn added a comment.EditedDec 3 2017, 9:38 AM

@bd808 thank you, I was on the wrong path where I already started looking back to the core code of MediaWiki to see what really does it return, it is indeed confusing.

I think the best way is to add an additional item in the config, so an alternative image can be set.

Change 394845 had a related patch set uploaded (by Divadsn; owner: Divadsn):
[mediawiki/skins/MinervaNeue@master] Use png as fallback when svg fails to load

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

divadsn added a comment.EditedDec 3 2017, 10:47 AM

I've decided to add a check instead, so it shouldn't make any difference on IE8 if the wiki doesn't have a png image version provided.

Edit: Works as intended :)

Just a note to say this is first on my review list tomorrow. Solution looks promising!!

divadsn added a comment.EditedDec 4 2017, 5:05 PM

@Jdlrobson cool, I think it shouldn't be a problem if there are 404 errors when the wiki hasn't a fallback png provided, in the end the IE user won't see anything.

I don't know if MediaWiki actually summarizes 404 not found errors, if yes then let me know so I will add a file exists check :)

Bawolff added a subscriber: Bawolff.Dec 4 2017, 5:16 PM

Wouldn't using srcset to set an svg (i.e. for the 1x size) be much nicer than using onerror handler?

divadsn added a comment.EditedDec 4 2017, 6:24 PM

@Bawolff indeed, and it would be much more cleaner to use srcset attribute as you have suggested on IRC :)

Change 394820 merged by jenkins-bot:
[operations/mediawiki-config@master] Add converted copyright svg images as png files

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

srcset is not supported by IE8 or IE11 so yeh that should probably work... https://caniuse.com/#search=srcset ... might help Opera Mini too.

The wordmarks are now deployed \o/:
https://en.m.wikipedia.org/static/images/mobile/copyright/wikipedia-wordmark-en.png

Uh cool, lemme only fix the issue on the other patch, as jenkins have failed a test :)

Change 394845 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Use png as fallback for svg on non-supported browsers

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

{F11181817}Thanks @divadsn for taking care of this! The change will roll out on the MediaWiki deploy train but is visible on the beta cluster!
https://en.m.wikipedia.beta.wmflabs.org/

Before:

After:

and special thanks @Bawolff for improving the solution :)

Jdlrobson closed this task as Resolved.

Great work @divadsn and thanks @Bawolff for accompanying the task!

divadsn added a subscriber: Krinkle.EditedDec 4 2017, 11:02 PM

@Krinkle has pointed out an issue I might have overlooked, will check it tomorrow and send another patch for that :)

Edit: Seems like the current solution is working fine as intended, a small PHP test have solved that issue.

echo pathinfo( "http://wikipedia.org/wikipedia-wordmark-en.svg.png", PATHINFO_EXTENSION );
> png

@Krinkle was referring to this:

> echo  str_replace( '.svg', '.png', 'https://en.wikipedia.org/static/images.svg/logo.svg.svg' );
> https://en.wikipedia.org/static/images.png/logo.png.png

Oh, now I get it, I'm too tired as you can see, will do the patch tomorrow, I promise :)

@divadsn I'm not seeing a patch. Did you get a chance to look at it?

@Jdlrobson, not yet, sorry, wasn't feeling good yesterday.

Change 395888 had a related patch set uploaded (by Divadsn; owner: Divadsn):
[mediawiki/skins/MinervaNeue@master] Use preg_replace instead of str_replace for replacing extension

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

Thanks a lot to @Krinkle for pointing out a bug which could cause problems in the future! :)

Change 395888 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Use preg_replace instead of str_replace for replacing extension

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