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?

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 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?

This is good to go.

Imported as https://codein.withgoogle.com/tasks/5313718495739904/

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.

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? :)

In T166684#3806453, @divadsn wrote:

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

@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 [[https://phabricator.wikimedia.org/diffusion/SMIN/browse/master/includes/skins/SkinMinerva.php;03ca369776371131cb4961def5cec8eae658eb34$871-924|SkinMinerva::prepareHeaderAndFooter()]] calls $tpl->set( 'footer-site-heading-html', $this->getSitename() );.
• [[https://phabricator.wikimedia.org/diffusion/SMIN/browse/master/includes/skins/SkinMinerva.php;03ca369776371131cb4961def5cec8eae658eb34$42-66|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.

@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

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!!

@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 :)

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

@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 :)

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

@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

Thanks @divadsn 👏👏