Last modified: 2014-09-22 21:56:49 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T32113, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 30113 - When sharing articles without images on social sites (facebook, twitter, google+) the mediawiki logo is used in lieu of site logo
When sharing articles without images on social sites (facebook, twitter, goog...
Status: PATCH_TO_REVIEW
Product: MediaWiki
Classification: Unclassified
Interface (Other open bugs)
1.23.0
Other All
: Low enhancement (vote)
: ---
Assigned To: Nobody - You can work on this!
http://lists.wikimedia.org/pipermail/...
: upstream
: 54830 59129 (view as bug list)
Depends on:
Blocks: 54829
  Show dependency treegraph
 
Reported: 2011-07-29 13:27 UTC by Dan Jacobson
Modified: 2014-09-22 21:56 UTC (History)
22 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
ugly logo extraction (122.50 KB, image/png)
2011-07-29 13:27 UTC, Dan Jacobson
Details
two terrible results of MediaWiki site logo degradation (49.33 KB, image/png)
2011-07-29 22:01 UTC, Dan Jacobson
Details

Description Dan Jacobson 2011-07-29 13:27:24 UTC
Created attachment 8838 [details]
ugly logo extraction

You duty at MediaWiki is to ensure that the user's site logo gets put in
the spot in the image instead of the MediaWiki logo.

Don't blame it on Facebook. You hid the user's site logo where Facebook
can't find it and put your logo where it could.

Don't blame it on the user and tell him to install plug-ins.

You have affected 100% of the wikis. Now please don't tell each user to
install plugins to fix it.

Please don't close this bug until this embarrassment is fixed.

What you are looking at is a link in a comment to a wall post, not the
first post, which of course you should please test too.
Comment 1 Sam Reed (reedy) 2011-07-29 14:45:46 UTC
(In reply to comment #0)
> Created attachment 8838 [details]
> ugly logo extraction
> 
> You duty at MediaWiki is to ensure that the user's site logo gets put in
> the spot in the image instead of the MediaWiki logo.
> 
> Don't blame it on Facebook. You hid the user's site logo where Facebook
> can't find it and put your logo where it could.
> 
> Don't blame it on the user and tell him to install plug-ins.
> 
> You have affected 100% of the wikis. Now please don't tell each user to
> install plugins to fix it.
> 
> Please don't close this bug until this embarrassment is fixed.
> 
> What you are looking at is a link in a comment to a wall post, not the
> first post, which of course you should please test too.

What?
Comment 2 Sam Reed (reedy) 2011-07-29 15:19:47 UTC
I'm not sure how this is a MediaWiki issue, the problem is with Facebooks Image scraping algorithm.

SURELY, it'd make sense for them to fix it in one place, rather than us fixing it and having to get that out to every wiki?
Comment 3 John Du Hart 2011-07-29 15:24:42 UTC
Really not our issue, try talking to Facebook about it. They should fix their image scraping code.
Comment 4 John Du Hart 2011-07-29 15:28:30 UTC
Addtionally in my test while I didn't get the Logo, I'm not getting the Powered By image. Reedy says he's getting the Logo on occasion.
Comment 5 Casey Brown 2011-07-29 15:35:39 UTC
Ignore Jidani's rude report and read the thread on wikitech-l. It's a lot more
constructive and gives more details and possible ways to fix it:
http://thread.gmane.org/gmane.org.wikimedia.mediawiki/37794

I think most people on there agreed there was something that we need to fix.
Comment 6 Dan Jacobson 2011-07-29 22:01:40 UTC
Created attachment 8844 [details]
two terrible results of MediaWiki site logo degradation

In Comment #4 you mention inconsistent results. However we see that a Post gets no logo, whereas a Comment gets the MediaWiki logo, with no other choice... all seen here on a vanilla MediaWiki wiki with just a simple site logo and no images.
Comment 7 Dan Jacobson 2011-07-29 22:03:48 UTC
(In reply to comment #5) Sorry but I'm steamed.
Since when in the TERMS does it say "you agree to have our ugly little
logo represent your site instead of your own painstakingly made logo".

Since when is the site logo not to be shown to anybody who doesn't use
an approved device, whereas e.g., Facebook and Google would never
dream of such a move. Even on Facebook's
  *  http://www.facebook.com/common/browser.php?_fb_noscript=1
  *  http://www.facebook.com/common/browser.php
they still make their logo available... accessible!
OK, this bugzilla page uses the same background image HACK to speed
loading or something. Fix that too!

Since when is the site logo not considered something that should be
available to text browsers, screen readers, whereas the "Powered by"
logo is? Nope, take them both to the grave if you insist, not just
one.

Accessibility 101. Or keep the HACK and add a <noscript> (hack2, if there is
no <nocss> to rescue the logo for text browsers. Yes, indeed, that way
you can keep your honor... you can reconcoct your CSS hack to
something like

<img src="$wgLogo" alt="$wgSitename" display:none><!--Sorry, never
studied CSS, and proud of it!-->
<current CSS hack to put logo in the background goes here>.
Comment 8 John Du Hart 2011-07-29 22:11:27 UTC
Jidanni being hostile and degrading towards developers will not entice anyone to fix this issue. An attitude like this is not conductive to getting any resolution for this issue.
Comment 9 John Du Hart 2011-07-29 22:15:20 UTC
Also, what exactly do you mean by "and all other devices"?
Comment 10 Dan Jacobson 2011-07-29 22:35:46 UTC
(In reply to comment #9)
> Also, what exactly do you mean by "and all other devices"?
Just make sure the site logo has no less [[Web accessibility]] than the MediaWiki Powered By logo. Thank you.
Comment 11 Dan Jacobson 2011-07-29 22:41:17 UTC
(In reply to comment #9) [[Device_Independence]] too. At least other devices will have some indication there was an <img> there. Even Facebook leaves an <img> on their 'your browser is not cool enough' page.
Comment 12 Dan Jacobson 2011-07-30 08:23:45 UTC
Let's check how some big name websites deal with the issue
$ w3m -dump http://www.flickr.com/ |sed q
Flickr logo. If you click it, you'll go home

Can't beat that.
Comment 13 Daniel Friesen 2011-07-30 08:27:08 UTC
...proprietary services don't have to deal with logos that overflow the logo area and cover the content area, various logo sizes that need to work without stretching or squashing, and a history of support for wiki that use a background-image inside their wiki's MediaWiki:*.css to change their logo in certain namespaces.
Comment 14 Dan Jacobson 2011-07-30 08:43:21 UTC
(In reply to comment #13)
Just say "you are allowed one aaaXbbb sized logo. Any other sizes and you are on your own. You shall bear the consequences." It's either that, or include some Flash image cropper crap.
Comment 15 Daniel Friesen 2011-07-30 09:08:54 UTC
...you do realize that in the three example wiki you listed on the mailing list as wiki you're trying to fix, all three of them use different logo sizes from each other, none of them the size we'd consider standard . Also you shot yourself in the foot with your second suggestion because you're trying to get the logo visible by FB, and if it's sitting in a Flash cropper FB isn't going to see it... you'll also kill accessibility for users without flash.

And this still doesn't address the regression that background-image -> img will cause to the wiki currently customizing their logo in specific sections of the wiki using css (many of them who likely can't change php). Examples I can find quickly are Uncyclopedia (see the UnNews section for example) and WikiNews' Portal:United_Kingdom.
Comment 16 Dan Jacobson 2011-07-30 09:51:25 UTC
I AM SURE that there is some way on earth to put a <img src> on a
MediaWiki page.

Call in all the astronauts in Houston. I'm sure we can do it.

Aim for that same upper left corner...

Don't ask me, I'm a HTML 4 strict guy with a non CSS http://jidanni.org/ website.

But how about my "add a <img src... display:none>[?] while keeping the
rest of what you do now intact" idea?

(I was talking about (flaky) Flash recroppers that kick in just once when
people upload avatars.)
Comment 17 Dan Collins 2011-07-31 01:15:20 UTC
So, the problem here is that facebook can't find a logo? What are 'all other devices', why is this mediawiki's fault rather than the other site, and why are you so terrible at making a lucid report?
Comment 18 Daniel Friesen 2011-07-31 01:25:03 UTC
(In reply to comment #17)
> So, the problem here is that facebook can't find a logo? What are 'all other
> devices', why is this mediawiki's fault rather than the other site, and why are
> you so terrible at making a lucid report?

We use a background-image for the logo instead of an <img> so Facebook can't find it when it scans the pages for images to use when sharing the article.

The various points here:
- <img> functions differently than a background-image:
-- background-image properly clips logos that are too large
-- We have existing wiki that are relying on the fact that the logo is a background-image and using site css to change the logo in various special cases like per-namespace
- It's possible to fix this in FB using OpenGraph, however og:image overrides the image list so including the logo there will break cases where FB is already finding good images, I'm also not sure people would like the idea of something nearly FB-specific being put into core
- Another suggestion is a redundant <img with display:none to hide it from everything else. It's a bit of a messy hack, waiting for other comments and reviews of that.

As for 'all other devices' jidanni is pretty much talking only about css-less text based browsers... Normal browsers handle it fine, mobile browsers handle it fine, screen readers can't display images, a good portion of the text browsers I know of don't display images anyways... I only remotely know of one text browser that shows images. I suppose the only category of people here affected are people with bad eyesight that specifically choose to use a text based browser that still displays images.
Comment 19 Dan Collins 2011-07-31 01:48:22 UTC
Thank you for the accurate summary. I wasn't aware that "text browser that shows images" was anything but an oxymoron, but if there are some, I suppose we should support them. I do know that we use a rather silly hack for no good reason. Would existing wikis that abuse CSS to change the logo per-namespace be able to do so if the logo was a pure image, or a background image with a second image set to display:none?
Comment 20 Dan Jacobson 2011-07-31 01:56:38 UTC
(In reply to comment #18)
I have good eyesight, but still use text browsers so I can write bad bug
reports :-)

In emacs_w3m, w3m, and lynx, a Proper Image will still leave a button
one can press if he wants to see the image. The perfect balance between
images bombardment and nothing.

MediaWiki however kidnaps the most important image on the whole site,
the theme setter: the site logo, into its background CSS hack, leaving
text browser users totally unaware that the wiki maintainer had bothered
to painstakingly create a site logo, or wish it was at least _accessible_.
Comment 21 Daniel Friesen 2011-07-31 01:59:08 UTC
(In reply to comment #19)
> Thank you for the accurate summary. I wasn't aware that "text browser that
> shows images" was anything but an oxymoron, but if there are some, I suppose we
> should support them. I do know that we use a rather silly hack for no good
> reason. Would existing wikis that abuse CSS to change the logo per-namespace be
> able to do so if the logo was a pure image, or a background image with a second
> image set to display:none?

It's not an oxymoron I suppose, but here it's only relevant to an extremely odd subset of users (potentially not even any), and the only thing 'broken' is that they don't see the logo. (Arguably not that important to someone who has forgone 99% of styling and dropped to a linear layout where the existence of a logo on every page could actually rob vertical space).

If the <img> is display:none; then the normal logo and the img should be mutually exclusive, the img will display when there is no css-support, and the logo will display when there is css-support.
The clearest caveat of course is that it would be redundant and a bit of a markup mess in every skin. Not sure if every other potential caveat is accounted for... It would also be completely useless redundancy if the wiki has the OpenGraphMeta extension installed.

For pure image, no. The image would always be in the way of the logo no matter where the bg is, so if we switched to <img> directly we'd regress the software taking all wikis that customize the logo that way and making them them display their original logo overlaying the logo they actually want. For opaque logos this will have the effect of covering the logo they want and making their css non-functional. For transparent logos of different outlines this could cause visual defects where the overlapping logos destroy each other, so to speak.
Comment 22 Casey Brown 2011-07-31 02:15:09 UTC
Daniel, you seem to be the one who knows the most about this and how to best handle it, so I'll ask you:

(In reply to comment #21)
> It would also be completely useless redundancy if the wiki has
> the OpenGraphMeta extension installed.

So should we just say "This is intended functionality. We offer an extension called OpenGraphMeta to accomplish this functionality if wiki-owners desire it. There's really no other easy way to do it without causing other problems for little gain." and close the bug?
Comment 23 Dan Jacobson 2011-07-31 02:20:30 UTC
(In reply to comment #21)
> we'd regress the software...
Back to the point where standard accessibility guidelines begun to be ignored,
and now we see the price that must be paid to re-engineer them back in at this
late stage.
(In reply to comment #22)
No. You have caused this bug for _all_ wikis. If it was merely a small number of wikis, you could tell them to install an extension and be done with it. But that's not what you have done.
Comment 24 Dan Collins 2011-07-31 02:57:48 UTC
First, to jidanni, I really think you'd get a fair bit further if you adopted a somewhat less accusative tone towards those who you're hoping will fix your problems for you.

Also, I hope I'm not the only one who thinks it's rather odd that we're trying to build an encyclopedia, but "the most important image on the whole site" is not an image used to illustrate the topic of an article, but the site's logo. I would hope that people who use our wiki are "painstaking[ly] craft[ing]" their content rather than a little pretty image.

As for an actual constructive suggestion, if the only reason we use the background hack in the first place is because it allows for images of the wrong size to be uploaded, maybe we should just require that uploaded images be the right size. Or use imagemagick, if available, to fix it. Then we could make our image be an image. Since presumably people making logos have some graphic design ability, they surely can crop their images.
Comment 25 Daniel Friesen 2011-07-31 04:03:06 UTC
A few more researched things to point out:

The display:none; <img> trick will only work for FB. Logos are links, as a result the link is the only actionable area, if a text browser /did/ make <img> tags actionable to view the image that functionality would be voided out by the fact that the logo is a link and would instead go to the main page. Which brings me to my next point.

jidanni's report on w3m and lynx's behavior on <img>s is false.
w3m and lynx DO NOT turn <img> tags into pressable buttons, and DO NOT provide the ability to open up the src of an <img> tag.
w3m and lynx take <img> tags and display the alt text as textual content, if there is no alt text nothing appears. Images including alt text are not made into links to the image.
The image behavior seams to be a false report based on the fact that in MediaWiki by default we wrap an image in an <a> that links to the File: page, and that File: page contains an <a> that points to the source image. When arriving on that source image the browser detects a mimetype it cannot handle and opens the default program for that mimetype. Which to a normal user's computer, opens the image viewer program. Said functionality is not necessarily intended for the consumption of images, but rather in the hopes that give documents such as a .pdf, .doc, or audio file a disabled person's computer will be able to handle the media using it's default program in a way that the disabled user can interact with.
So, opening an image associated with a <img>... not a text browser feature. Being able to view a logo from a text browser, not functionality expected from a text browser browsing the standard accessible web.

Additionally I've been reviewing that Web accessibility page on WP, and the w3c links... I don't believe that accessibility standards consider the use of text based browsers by users who are capable of seeing to be an accessibility target.
Users who cannot see and require the use of a screen reader for braille or auditory output are included of course. But users who are only have bad eyesight appear to be included not in the context of css-less text browsers, but in the context of full css-capable web browsers, with tweaks to have them display content larger. Which we DO actually have support for in ways some other websites don't (we use em's specifically for these users).


So the claims that the ability to access the wiki's logo image from a text browser is relevant to web accessibility... those arguments are looking pretty void right now. Users who can actually see a logo should be using a css capable web browser.

This bug looks limited to "Please make the logo show up in Facebook" now. Which naturally was NOT relevant when our practice of using a background-image started, as that started in MonoBook (actually no, pre monobook), and back then Facebook did not exist.
Comment 26 Chad H. 2011-07-31 04:05:32 UTC
(In reply to comment #23)
> No. You have caused this bug for _all_ wikis. If it was merely a small number
> of wikis, you could tell them to install an extension and be done with it. But
> that's not what you have done.

It's not affecting all wikis...only wikis who wish to share their content of Facebook. For example: it affects none of my wikis.
Comment 27 Daniel Friesen 2011-07-31 04:30:28 UTC
(In reply to comment #24)
> Also, I hope I'm not the only one who thinks it's rather odd that we're trying
> to build an encyclopedia, but "the most important image on the whole site" is
> not an image used to illustrate the topic of an article, but the site's logo. I
> would hope that people who use our wiki are "painstaking[ly] craft[ing]" their
> content rather than a little pretty image.
Heh, I was thinking of saying something along those lines too. A wiki where the logo is "painstaking[ly] crafted" but the content is not "painstaking[ly] crafted" is worthless. And we do make an effort to make sure that as much of that painstakingly crafted content is accessible to disabled users.

(In reply to comment #24)
> As for an actual constructive suggestion, if the only reason we use the
> background hack in the first place is because it allows for images of the wrong
> size to be uploaded, maybe we should just require that uploaded images be the
> right size. Or use imagemagick, if available, to fix it. Then we could make our
> image be an image. Since presumably people making logos have some graphic
> design ability, they surely can crop their images.

That I know of there are 3 current functional uses of the logo being a background-image:
- Without any special coding a background-image that is too large gets cut off (we have skins with various logo area sizes, and the logo can be any arbitrary url to any unvetted image), unlike an <img> which if allowed to grow will overlap the content area. (note that since $wgLogo is an arbitrary url we can't do any ImageMagic stuff with it, iirc MediaWiki also functions mostly in hosts with url fopen wrappers disabled, so we also can't do anything that would require MW to download an image from a url)
- Using a background-image with a center center position as we do means we cater to a variety of logo sizes while still perfectly centering them in all skins.
- Using a background-image also functions as a way to allow the logo and the logo area to be customized in various ways by css.

I did some testing to see what can be done with an <img> instead of the background-image. With use of overflow: hidden; it is possible to have the logo area cut off overly large logos easily. The centering however is a bit of good and bad. A text-align: center; does permit the logo to be horizontally centered like the background-image. However we cannot vertically center a logo that is an <img> like we can for a background-image. With some tweaks to the css it is possible to instead make the logo area vertically shrink to fit the logo of whatever height. However this has two caveats, both I'm sure we'll end up with people complaining about. A) Of course, the wiki with vertically small logos may not be happy about their logo area suddenly changing size. Technically they can pad their logos or hardcode margins. However that's not going to make wiki with extra skins including ones where a vertically conservative logo is a good thing happy. And forcing people to hardcode px margins is not a good practice for us (actually if they don't enable site stylesheets they can't even do that without code hacks, another bad tradeoff). B) More importantly, while the logo hasn't loaded the navigation will appear higher up in the sidebar, and once it's loaded will move to accommodate the logo. This will have the effect of making the sidebar jump around, create a visual distraction during the loading of the page, and can actually create a usability defect where a user goes to click a link, the logo loads, the sidebar jumps, and then the user clicks a different link. (this problem will be particularly noticed by low-speed Internet users)
Comment 28 Daniel Friesen 2011-07-31 04:44:32 UTC
Given the fact that this doesn't appear to actually be relevant to web accessibility. And given the fact that the background-image practice existed before Facebook existed when there was no reason to not use a background-image for a presentational feature like the logo, this appears to be an enhancement related to Facebook rather than a bug. And given the fact that Facebook is not a high priority for core, and an extension already provides functionality for wiki that desire extra Facebook compatibility this seams to be a low priority core enhancement.
Comment 29 p858snake 2011-07-31 04:46:43 UTC
+Upstream, this is a issue with facebook, not us.
Comment 30 Dan Collins 2011-07-31 04:47:38 UTC
Wouldn't that be downstream?
Comment 31 Casey Brown 2011-07-31 04:49:25 UTC
I think comment 28 is a perfectly fine "WONTFIX" or "WORKSFORME" response, rather than a downgrade to "low priority". If it's not causing a problem and we have an extension to fix this, and we don't want it to go into core, than this can probably be closed.
Comment 32 Daniel Friesen 2011-07-31 04:52:12 UTC
(In reply to comment #29)
> +Upstream, this is a issue with facebook, not us.
(In reply to comment #30)
> Wouldn't that be downstream?

Upstream or downstream, Facebook technically already provides support for a
metadata standard.
It's more of a "We just know that we're going to get angry users if we include
Facebook related metadata into core" issue.
Also a "Using $wgLogo in og:image has inadvertent negative side effects that require non-trivial extra coding so we can't just drop it in anyways."
Comment 33 Dan Jacobson 2011-07-31 04:59:19 UTC
(In reply to comment #25)
> w3m and lynx DO NOT...
All I know is I can tell they are there
$ echo '<img src="zzz">'|w3m -dump -T text/html
[zzz]
and I can hit
> t runs the command w3m-toggle-inline-image, which is an interactive
> compiled Lisp function in `w3m.el'. [emacs_w3m]
But of course not with the wiki logo. But of course real men are supposed to
use real browsers, so never mind.
P.S., Go ahead and close the bug. But like Ernest said, "I like the little logo",
http://www.youtube.com/watch?v=v8aVKTVCJpM
Comment 34 Dan Collins 2011-07-31 05:02:02 UTC
Why would you want to know that the logo is there on a text browser anyway? Seems like it just adds noise?

Anyway, closing per comment 28, comment 31, and comment 33 as invalid.
Comment 35 Daniel Friesen 2011-07-31 05:05:32 UTC
(In reply to comment #33)
> (In reply to comment #25)
> > w3m and lynx DO NOT...
> All I know is I can tell they are there
> $ echo '<img src="zzz">'|w3m -dump -T text/html
> [zzz]
> and I can hit
> > t runs the command w3m-toggle-inline-image, which is an interactive
> > compiled Lisp function in `w3m.el'. [emacs_w3m]
> But of course not with the wiki logo. But of course real men are supposed to
> use real browsers, so never mind.
Text editors are not part of our compatibility guidelines: [[mw:Browser support]] 

Rather... I can't think of any website that would sanely consider a text editor a compat target.
Comment 36 Chad H. 2013-10-15 03:01:17 UTC
*** Bug 54830 has been marked as a duplicate of this bug. ***
Comment 37 Chad H. 2013-10-15 03:01:57 UTC
Reopening after the dupe from 54830.
Comment 38 Quim Gil 2013-12-04 17:36:31 UTC
I just want to mention that this problem affects not only Facebook users sharing URLs of MediaWiki sites, also Google+ users, and probably any other service featuring URLs with an image retrieved automatically. When those pages have no images, these services take automatically the little ugly "Powered by MediaWiki" banner in the footer, or nothing if it has been removed. The prominent site logo is always ignored.

As someone sharing frequently MediaWiki pages in so-called social networks, I believe this problem is being underestimated here. These services are currently the main channel of promotion of URLs, and these services feature prominently the main image contained in those URLs because they make the link more attractive. If you want to promote a MediaWiki page without graphics you will be totally out of luck: there will be no way to use at least your site logo.

Meanwhile, other wikis and CMSs...
Comment 40 Bartosz Dziewoński 2013-12-04 19:46:54 UTC
Bug 29242 – nope, entirely different thing. Bug 31338 sounds related, maybe that approach could be used here (but I don't know enough about this stuff to judge).

For listeners tuning in now, the reasons again regular <img src> were outlined in comment 27.
Comment 41 Jared Zimmerman (WMF) 2013-12-04 19:52:59 UTC
Can someone make an umbrella tracking bug for "Better presentation of content shared via mediawiki on external sites" I want to make sure whoever works on this doesn't miss anything.
Comment 42 Bartosz Dziewoński 2013-12-04 21:21:13 UTC
What bugs would that track? If you have a list, just create one.
Comment 43 Quim Gil 2013-12-04 21:28:54 UTC
About comment #27

Ref wiki pages without images being bad. True for Wikipedia, not for all the MediaWikis out there. And even without leaving the Wikimedia context: what about Wiktionary, Wikiquotes, Wikisource...? 

Ref wrong images becoming site logos. Isn't that problem obvious when it happens, and isn't the first interest of a sane admin to make sure the logo looks good? I don't see why we should protect admins from themselves. Cutting an image right is not that difficult.

This leaves us purely with the technical problems described when using <img src>. I'm not saying they are not relevant, I just want to focus the discussion in the technical arguments, leaving aside the rest.
Comment 44 Daniel Friesen 2013-12-04 21:54:07 UTC
My comment? Could you be a little more specific about which parts of the comment you're referencing, I'm having a little bit of trouble matching them up to parts of my comment.
Comment 45 Quim Gil 2013-12-05 21:36:29 UTC
I started with default quotes, but the comment was getting really long. Let me try harder:

> (In reply to comment #24)
>> Also, I hope I'm not the only one who thinks it's rather odd that we're trying
>> to build an encyclopedia, but "the most important image on the whole site" is
>> not an image used to illustrate the topic of an article, but the site's logo. I
>> would hope that people who use our wiki are "painstaking[ly] craft[ing]" their
>> content rather than a little pretty image.
> Heh, I was thinking of saying something along those lines too. A wiki where the
> logo is "painstaking[ly] crafted" but the content is not "painstaking[ly]
> crafted" is worthless. 

True for Wikipedia, not for all the MediaWikis out there. And even without leaving the Wikimedia context: what about Wiktionary, Wikiquotes, Wikisource...? 

We can't assume that the quality, completeness, and interest of a wiki page is measured by the existence of an illustrative image or not. 


> That I know of there are 3 current functional uses of the logo being a
> background-image:
> - Without any special coding a background-image that is too large gets cut off
> (we have skins with various logo area sizes, and the logo can be any arbitrary
> url to any unvetted image), unlike an <img> which if allowed to grow will
> overlap the content area. (note that since $wgLogo is an arbitrary url we can't
> do any ImageMagic stuff with it, iirc MediaWiki also functions mostly in hosts
> with url fopen wrappers disabled, so we also can't do anything that would
> require MW to download an image from a url)
> - Using a background-image with a center center position as we do means we
> cater to a variety of logo sizes while still perfectly centering them in all
> skins.
> - Using a background-image also functions as a way to allow the logo and the
> logo area to be customized in various ways by css.

I don't see why we should protect admins from themselves. Cutting an image right is not that difficult.

Our documentation currently says that a logo is expected to be 135x135. Admins playing with other sizes (locally or remotely) do it at ther own risk.

https://www.mediawiki.org/wiki/Manual:$wgLogo


> I did some testing to see what can be done with an <img> instead of the
> background-image. With use of overflow: hidden; it is possible to have the logo
> area cut off overly large logos easily.

Good! One problem less.

> The centering however is a bit of good
> and bad. A text-align: center; does permit the logo to be horizontally centered
> like the background-image. However we cannot vertically center a logo that is
> an <img> like we can for a background-image. With some tweaks to the css it is
> possible to instead make the logo area vertically shrink to fit the logo of
> whatever height. However this has two caveats, both I'm sure we'll end up with
> people complaining about. A) Of course, the wiki with vertically small logos
> may not be happy about their logo area suddenly changing size. Technically they
> can pad their logos or hardcode margins. However that's not going to make wiki
> with extra skins including ones where a vertically conservative logo is a good
> thing happy. And forcing people to hardcode px margins is not a good practice
> for us (actually if they don't enable site stylesheets they can't even do that
> without code hacks, another bad tradeoff). 

If your logo is expected to be 135x135, then you can place your vertically small logo within a square canvas leaving the rest transparent.

> B) More importantly, while the logo
> hasn't loaded the navigation will appear higher up in the sidebar, and once
> it's loaded will move to accommodate the logo. This will have the effect of
> making the sidebar jump around, create a visual distraction during the loading
> of the page, and can actually create a usability defect where a user goes to
> click a link, the logo loads, the sidebar jumps, and then the user clicks a
> different link. (this problem will be particularly noticed by low-speed
> Internet users)

Mmm even if you are placing that image inside a fixed size div, in combination with the mentioned "overflow: hidden"?
Comment 46 Daniel Friesen 2013-12-06 00:40:46 UTC
(In reply to comment #45)
> > Heh, I was thinking of saying something along those lines too. A wiki where the
> > logo is "painstaking[ly] crafted" but the content is not "painstaking[ly]
> > crafted" is worthless. 
> 
> True for Wikipedia, not for all the MediaWikis out there. And even without
> leaving the Wikimedia context: what about Wiktionary, Wikiquotes,
> Wikisource...? 
> 
> We can't assume that the quality, completeness, and interest of a wiki page
> is
> measured by the existence of an illustrative image or not.

I guess I personally glossed over the "image used to illustrate the topic of an article" part. When I said "content" I literally meant content. I view the logo as a superfluous decoration, what's important is the content.
Tbh, on a wiki like Wiktionary where pages don't have images I don't really think of the logo as a substitute for them. IMHO pages like those that are text-only and don't have a representative image are typically best shared just like that, as text-only shares.


> I don't see why we should protect admins from themselves. Cutting an image
> right is not that difficult.
> 
> Our documentation currently says that a logo is expected to be 135x135.
> Admins
> playing with other sizes (locally or remotely) do it at ther own risk.
> 
> https://www.mediawiki.org/wiki/Manual:$wgLogo

Our recommendation for logo size is pretty much just pulled out of a hat, we've never had a true standard for image sizes.
- In one of MW's first skins, Nostalgia, the logo was simply inserted into the corner as an <img> there was no fixed sidebar area content simply floated around the logo, you could use absolutely any logo size you wanted.
- In Standard/Classic the logo area was somewhere around 135px in height.
- MonoBook's logo area is 155px in height.
- And Vector's logo area is 160px in height.
- And we have no clue what size of logo area other skins use.

Now take note of how I stated "###px in height", I mean that quite literally. Our logos only guarantee a specific number of pixels for the height. While by default the width typically matches the same number of pixels as the height, our logo area widths are actually defined as ##em numbers. They are explicitly set so that the width of the logo area can both grow beyond what you'd expect and shrink to smaller than it is by default.

So our first skins had no restrictions on logo sizes, our default skins have been changing the restriction each time we make a new one, the width is actually not fixed to a pixel size.

I can also confirm that site authors don't care about what our manual says about $wgLogo. They simply create whatever fits in the logo area on their default skin. And that number grows, back when Wikia had monobook as a default I made plenty of 150x150px and 155x155px logos.

Rather than protecting site authors, cutting off logos is a little more about protecting site users using non-default skins.

There's also the whole bit about customizing the logo by site css. A number of wikis do this in practice. en.wp does it in Common.css to provide mdpi and hdpi logos. And other wikis do it overriding the default logo in some namespaces, etc...


> If your logo is expected to be 135x135, then you can place your vertically
> small logo within a square canvas leaving the rest transparent.
> 
> ...
> 
> Mmm even if you are placing that image inside a fixed size div, in
> combination
> with the mentioned "overflow: hidden"?

A fixed size div will fix the jump, but it won't fix the vertical centering of the logo. We can't know the height of the logo beforehand. And fixing the height will simply leave the logo at the top of that region. So small logos still won't be vertically centered.
Comment 47 Dan Jacobson 2013-12-06 03:31:13 UTC
> You are receiving this mail because:

>  * You reported the bug.
OK that is very nice, but there is no way for me to unsubscribe due to bug 58066 so everybody please stop talking until then.
Comment 48 Bawolff (Brian Wolff) 2013-12-06 03:39:29 UTC
(In reply to comment #47)
> > You are receiving this mail because:
> 
> >  * You reported the bug.
> OK that is very nice, but there is no way for me to unsubscribe due to bug
> 58066 so everybody please stop talking until then.

Umm no. Provided comments are on topic, people are going to comment on bug as needed in order to work towards a resolution. If you don't like it, you can:

*Change your email prefs in https://bugzilla.wikimedia.org/userprefs.cgi?tab=email to not email for bugs you created
*Set up an email filter
*Delete the email without reading it
etc
Comment 49 Dan Jacobson 2013-12-06 03:56:02 UTC
(In reply to comment #48)
In fresh Bugzilla it would take me just one click to unsubscribe, just like the CC people.
Alas, with this old Bugzilla, one needs to make a Spamassassin Filter for just this one bug, lest one mess up ones other settings.

OK I'll use your third option. Certainly this can't last forever.
Comment 50 Andre Klapper 2013-12-06 10:33:20 UTC
jidanni: That's really stuff to discuss in bug 49597 (plus your statement is wrong as we will not run unstable development versions in production.) 
Feel free to create a bug report here asking to backport the 4.5 fix for https://bugzilla.mozilla.org/show_bug.cgi?id=148564 once we've upgraded to 4.4.
Comment 51 Andre Klapper 2014-01-09 14:02:25 UTC
*** Bug 59129 has been marked as a duplicate of this bug. ***
Comment 52 Jared Zimmerman (WMF) 2014-01-09 19:27:57 UTC
In case it wasn't clear from the the discussion above, this only seems to be an issues for article that are shared which do not have any images. 

When sharing through a link to a page on wikipedia or other media wiki sites
the software usually correctly picks the lead or first image from an article,
however if the article does not contain any images or only transcluded images .
e.g. https://en.wikipedia.org/wiki/List_of_Colorado_ski_resorts then the
mediawiki logo from the site footer is used instead. (see attached screenshot)
instead of the mediawiki logo, the site logo (at a sufficiently high
resolution) should be used instead, as it would be a more relevant to have the
site logo (where the link goes to) rather than the logo of the CMS the content
is displayed through.
Comment 53 Wes Turner 2014-01-18 17:37:02 UTC
I see a few possible solutions to this problem:

1. Make the Wikipedia logo an <img> tag

    * Not preferable because accessibility.

2. Make the MediaWiki logo a background-image property of a <div> (or <a>) tag

    * Probably the easiest thing to do.

3. Add an additional <img> tag with the Wikipedia logo (e.g. to the footer)

    * Additional [cacheable] image : https://upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png

4. Specify og: metadata in page headers

    * http://ogp.me/#structured : og:image, og:secure_image
    
    * I don't know how/whether this takes priority over images found in-page for sites such as Facebook and Google+


TBH, the accessibility argument would also apply to unnecessary <img> tags (with @alt attributes) in the footer.
Comment 54 Daniel Friesen 2014-01-18 17:55:35 UTC
(In reply to comment #53)
> 2. Make the MediaWiki logo a background-image property of a <div> (or <a>)
> tag

Once upon a time, Facebook's documentation said that all images smaller than 50x50px were excluded from being used as images. Tbh if they actually stuck to that this wouldn't even be a problem.

> 4. Specify og: metadata in page headers
> 
>     * http://ogp.me/#structured : og:image, og:secure_image
> 
>     * I don't know how/whether this takes priority over images found in-page
> for sites such as Facebook and Google+

Yes, that's generally the point of the metadata, it has precedence over in-page images.
Comment 55 Wes Turner 2014-01-18 18:24:00 UTC
Not cool when you're trying to share links to non-gender-specific Wikipedia things like motorcycles and stuff and there's a beautiful MediaWiki flower there and you have to choose between the inline summary and the flower image; I'm waiting for the 400MB+ MediaWiki repository to clone so I can edit the HTML in the footer.
Comment 56 Wes Turner 2014-01-18 18:28:22 UTC

(In reply to comment #54)
> (In reply to comment #53)
> > 2. Make the MediaWiki logo a background-image property of a <div> (or <a>)
> > tag
> 
> Once upon a time, Facebook's documentation said that all images smaller than
> 50x50px were excluded from being used as images. Tbh if they actually stuck
> to
> that this wouldn't even be a problem.
> 
> > 4. Specify og: metadata in page headers
> > 
> >     * http://ogp.me/#structured : og:image, og:secure_image
> > 
> >     * I don't know how/whether this takes priority over images found in-page
> > for sites such as Facebook and Google+
> 
> Yes, that's generally the point of the metadata, it has precedence over
> in-page
> images.

Okay, so #4 isn't feasible because specifying og:image would preempt image selection.

#2: I understand that setting opacity: 0 with background-image is acceptable as a replacement for (incomplete) @alt attributes on <img> tags. [1]

[1] https://stackoverflow.com/questions/10009751/background-images-over-hidden-text-bad-for-accessibility

#3: We may need some help with obtaining a Wikipedia image with a similar aspect ratio (88x31)
Comment 57 Wes Turner 2014-01-18 18:38:05 UTC
Why isn't there an option to select between "a Wikimedia project" and "Powered by MediaWiki" if both images are 88x31?

Not too familiar with the mediawiki codebase [...]

   $ grin -i poweredby

   $ grin 'getPoweredBy|poweredbyico' . -C 5

I see a getPoweredBy function in includes/Skin.php :

    $url = htmlspecialchars( "$wgStylePath/common/images/poweredby_mediawiki_88x31.png" );

And then in includes/SkinTemplate.php a template variable is set:

    $tpl->set( 'poweredbyico', $this->getPoweredBy() );

But how/where does 'poweredbyico' get included into the Vector template?
Comment 58 Wes Turner 2014-01-18 18:41:27 UTC
(BTW I'm following this tutorial in order to learn how to submit a patch to fix this issue:  https://www.mediawiki.org/wiki/Gerrit/Tutorial#How_to_submit_a_patch )
Comment 59 Wes Turner 2014-01-18 18:48:48 UTC
#2: In includes/Skin.php, I believe the following approach to getPoweredBy() would be accessible and ___-COMPLIANT.

 "<a href='http://www.mediawiki.org/'><img src='$url' alt='MediaWiki' /></a>";

"<a href='http://www.mediawiki.org/'><span style="background-image: url('$url'); width: 88px; height: 31px; opacity: 0">trans(Powered by) MediaWiki</a>"

I'm not sure why we wouldn't need to do the same for getCopyrightIcon().
Comment 60 Wes Turner 2014-01-18 18:53:04 UTC
As an aside, with RDFa, we could probably just add `property="og:image"` to all of the in-page images.
Comment 61 Wes Turner 2014-01-18 18:58:20 UTC
... For http://schema.org/Organization (which can be expressed in RDFa), we could specify `property="logo" content="https://upload.wikimedia.org/wikipedia/en/b/bc/Wiki.png"`
Comment 62 Daniel Friesen 2014-01-18 19:38:55 UTC
(In reply to comment #57)
> Why isn't there an option to select between "a Wikimedia project" and
> "Powered
> by MediaWiki" if both images are 88x31?

Unfortunately you'll have to ask Facebook that, it's their proprietary code that's picking the images out of standardized html.

> But how/where does 'poweredbyico' get included into the Vector template?

poweredbyico is deprecated, skins now use footericons.
See https://www.mediawiki.org/wiki/Manual:$wgFooterIcons

(In reply to comment #60)
> As an aside, with RDFa, we could probably just add `property="og:image"` to
> all
> of the in-page images.

OGP isn't RDFa, they do state "We've based the initial version of the protocol on RDFa" but it isn't actual RDFa and it violates some of RDFa's rules. OGP's "spec" only accepts data from meta tags in the head.

However I have considered extracting the list of images included in the page output and outputting og:image data in the head for them. Actually I've had other ideas like allowing images in content to be marked as presentational/non-presentational so we can intelligently exclude bad images.
Comment 63 Wes Turner 2014-01-18 19:44:33 UTC
I've submitted a preliminary patch to master to gerrit which implements #2 (with @title attributes instead of opacity:0)

https://gerrit.wikimedia.org/r/#/c/108221/
Comment 64 Wes Turner 2014-01-18 19:55:20 UTC
(In reply to comment #62)
> (In reply to comment #57)
> > Why isn't there an option to select between "a Wikimedia project" and
> > "Powered
> > by MediaWiki" if both images are 88x31?
> 
> Unfortunately you'll have to ask Facebook that, it's their proprietary code
> that's picking the images out of standardized html.
> 
> > But how/where does 'poweredbyico' get included into the Vector template?
> 
> poweredbyico is deprecated, skins now use footericons.
> See https://www.mediawiki.org/wiki/Manual:$wgFooterIcons
> 
> (In reply to comment #60)
> > As an aside, with RDFa, we could probably just add `property="og:image"` to
> > all
> > of the in-page images.
> 
> OGP isn't RDFa, they do state "We've based the initial version of the
> protocol
> on RDFa" but it isn't actual RDFa and it violates some of RDFa's rules. OGP's
> "spec" only accepts data from meta tags in the head.

Ah. Good call.

http://schema.org/WebPage .image

http://schema.org/WebPage .primaryImageOfPage

http://schema.org/ImageObject



> 
> However I have considered extracting the list of images included in the page
> output and outputting og:image data in the head for them. Actually I've had
> other ideas like allowing images in content to be marked as
> presentational/non-presentational so we can intelligently exclude bad images.

TIL about https://www.mediawiki.org/wiki/Extension:OpenGraphMeta
Comment 65 Wes Turner 2014-01-18 19:56:02 UTC
* schema:Thing has a schema:image property
Comment 66 Wes Turner 2014-01-18 19:58:33 UTC
You're probably already aware of https://www.wikidata.org/wiki/Property:P18 (image) and https://www.wikidata.org/wiki/Property:P154 (logo image).
Comment 67 Wes Turner 2014-01-18 20:08:14 UTC
... Looking at makeFooterIcon() in includes/Skin.php (I had 'tested' the raw HTML w/ Chrome): is there a fancy way to specify inline CSS style attributes (like background-image, width, height, and display) with Html::element or Html::rawElement? (Which templating library is this?)
Comment 68 Daniel Friesen 2014-01-18 20:52:52 UTC
(In reply to comment #64)
> > However I have considered extracting the list of images included in the page
> > output and outputting og:image data in the head for them. Actually I've had
> > other ideas like allowing images in content to be marked as
> > presentational/non-presentational so we can intelligently exclude bad images.
> 
> TIL about https://www.mediawiki.org/wiki/Extension:OpenGraphMeta

I've actually been hoping to make this metadata part of core and deprecating OpenGraphMeta. This could all be done much better if re-implemented as part of core. And this really is basic stuff no wiki should be without.

Though that depends on stuff implemented alongside this RFC:
https://www.mediawiki.org/wiki/Requests_for_comment/Update_our_code_to_use_RDFa_1.1_instead_of_RDFa_1.0
Comment 69 Daniel Friesen 2014-01-18 20:55:01 UTC
(In reply to comment #67)
> ... Looking at makeFooterIcon() in includes/Skin.php (I had 'tested' the raw
> HTML w/ Chrome): is there a fancy way to specify inline CSS style attributes
> (like background-image, width, height, and display) with Html::element or
> Html::rawElement? (Which templating library is this?)

Nope, we don't have such a thing right now. And you'll probably want to consider using the css sanitizing method inside Sanitize:: for stuff besides the background image and checking that nothing tries crossing a ;.
Comment 70 Subfader 2014-01-18 20:58:37 UTC
I hacked the logo as first og:image into the header, followed by all other article images. I see no disadvantages so far.
Comment 71 Wes Turner 2014-01-18 21:17:16 UTC
(In reply to comment #69)
> (In reply to comment #67)
> > ... Looking at makeFooterIcon() in includes/Skin.php (I had 'tested' the raw
> > HTML w/ Chrome): is there a fancy way to specify inline CSS style attributes
> > (like background-image, width, height, and display) with Html::element or
> > Html::rawElement? (Which templating library is this?)
> 
> Nope, we don't have such a thing right now. And you'll probably want to
> consider using the css sanitizing method inside Sanitize:: for stuff besides
> the background image and checking that nothing tries crossing a ;.

It's been awhile since I've written any PHP; I'll have to lookup what htmlspecialchars does again. Are there test cases for these? It may be better for someone familiar with the input sanitization codebase to tkae it from here.

Where in the docs would I look for 'trusted' configuration settings? Can these variables be modified?

It looks like Sanitizer::validateAttributes would call Sanitizer::checkCss on the style property; but the docstrings for Sanitizer::checkCss specify:

    * Currently URL references, 'expression', 'tps' are forbidden.

so I suppose the following would be needed:

* background-image: '";>
* width: safeEncodeAttribute
* height: safeEncodeAttribute
Comment 72 Wes Turner 2014-01-18 21:27:45 UTC
(In reply to comment #70)
> I hacked the logo as first og:image into the header, followed by all other
> article images. I see no disadvantages so far.

Cool! Which networks does it work w/?
Comment 73 Subfader 2014-01-18 21:59:34 UTC
Wes: I only tested for Facebook and debugged using https://developers.facebook.com/tools/debug/
Comment 74 Wes Turner 2014-01-18 22:03:22 UTC
(In reply to comment #73)
> Wes: I only tested for Facebook and debugged using
> https://developers.facebook.com/tools/debug/

Nice. So Facebook does parse (some?) og:image tags beyond just <meta> tags within <head>?

https://stackoverflow.com/questions/10397510/are-open-graph-tags-just-for-facebook
Comment 75 Wes Turner 2014-01-18 22:05:56 UTC
(In reply to comment #68)
> (In reply to comment #64)
> > > However I have considered extracting the list of images included in the page
> > > output and outputting og:image data in the head for them. Actually I've had
> > > other ideas like allowing images in content to be marked as
> > > presentational/non-presentational so we can intelligently exclude bad images.
> > 
> > TIL about https://www.mediawiki.org/wiki/Extension:OpenGraphMeta
> 
> I've actually been hoping to make this metadata part of core and deprecating
> OpenGraphMeta. This could all be done much better if re-implemented as part
> of
> core. And this really is basic stuff no wiki should be without.
> 
> Though that depends on stuff implemented alongside this RFC:
> https://www.mediawiki.org/wiki/Requests_for_comment/
> Update_our_code_to_use_RDFa_1.1_instead_of_RDFa_1.0

Did anyone mention an RDFa 1.0 requirement?

I've been parsing / checking / validating with PyRDFa.
Comment 76 Daniel Friesen 2014-01-19 04:18:42 UTC
(In reply to comment #71)
> It looks like Sanitizer::validateAttributes would call Sanitizer::checkCss on
> the style property; but the docstrings for Sanitizer::checkCss specify:
> 
>     * Currently URL references, 'expression', 'tps' are forbidden.
> 
> so I suppose the following would be needed:
> 
> * background-image: '";>
> * width: safeEncodeAttribute
> * height: safeEncodeAttribute

Html::element already handles encoding attributes, you just want to sanitize css.(In reply to comment #74)

> (In reply to comment #73)
> Nice. So Facebook does parse (some?) og:image tags beyond just <meta> tags
> within <head>?
> 
> https://stackoverflow.com/questions/10397510/are-open-graph-tags-just-for-
> facebook

Besides Google+ there's also a list of OGP parsers to test:
http://ogp.me/#implementations

(In reply to comment #75)
> (In reply to comment #68)
> Did anyone mention an RDFa 1.0 requirement?

The commit that implements that RFC includes a high level API for managing RDFa prefixes, adding them to <html>, and adding RDFa to the <head>.

$og = $out->getPrefixContext()->prefix( 'og', 'http://ogp.me/ns#', true );
$out->addGraphProperty( $og->curie( 'image' ), ... );
Comment 77 Wes Turner 2014-01-19 04:49:18 UTC
(In reply to comment #76)
> (In reply to comment #71)
> > It looks like Sanitizer::validateAttributes would call Sanitizer::checkCss on
> > the style property; but the docstrings for Sanitizer::checkCss specify:
> > 
> >     * Currently URL references, 'expression', 'tps' are forbidden.
> > 
> > so I suppose the following would be needed:
> > 
> > * background-image: '";>
> > * width: safeEncodeAttribute
> > * height: safeEncodeAttribute
> 
> Html::element already handles encoding attributes, you just want to sanitize
> css.(In reply to comment #74)

This test fixture seems to indicate that a

     'background-image: url(' . $wgFooterIcons['powererdby']['src'] . ')';

CSS attribute containing input from configuration on the filesystem may be stripped:

https://git.wikimedia.org/blob/mediawiki%2Fcore/6a2d25eed09c311c70657789b3f7a841bc5363db/tests%2Fphpunit%2Fincludes%2FSanitizerTest.php#L253

https://www.mediawiki.org/wiki/Manual:$wgFooterIcons

HTML::element states:

			// There's no point in escaping quotes, >, etc. in the contents of
			// elements.

In this case, it is probably good to escape a ' and/or javascript: in the configuration-supplied variable.

It would be helpful if someone more familiar with the codebase could indicate if there is a more appropriate function than htmlspecialchars (with ENT_NOQUOTES/ENT_QUOTES) for this.
Comment 78 Wes Turner 2014-01-19 04:52:50 UTC
* In this case, it is probably good to escape a '"; and/or javascript: in the
configuration-supplied variable.
Comment 79 Daniel Friesen 2014-01-19 05:03:35 UTC
(In reply to comment #77)
> This test fixture seems to indicate that a
> 
>      'background-image: url(' . $wgFooterIcons['powererdby']['src'] . ')';
> 
> CSS attribute containing input from configuration on the filesystem may be
> stripped:
> 
> https://git.wikimedia.org/blob/mediawiki%2Fcore/
> 6a2d25eed09c311c70657789b3f7a841bc5363db/
> tests%2Fphpunit%2Fincludes%2FSanitizerTest.php#L253

Yes, all url() constructs will make checkCss consider the css unsafe. So you'll have to sanitize the background-image separate from the rest of the css.

> HTML::element states:
> 
>             // There's no point in escaping quotes, >, etc. in the contents
> of
>             // elements.
> 
> In this case, it is probably good to escape a '"; and/or javascript: in the
> configuration-supplied variable.
> 
> It would be helpful if someone more familiar with the codebase could indicate
> if there is a more appropriate function than htmlspecialchars (with
> ENT_NOQUOTES/ENT_QUOTES) for this.

Escaping of quotes ('") are already handled by Html::element, ignore htmlspecialchars completely here.

For sanitizing the URL is safe/not javascript: use preg_match( '/^(' . wfUrlProtocols() . ')[^\s]+$/', ... ) to test if the protocol is whitelisted.
Comment 80 Daniel Friesen 2014-01-19 05:06:08 UTC
It's a shame that Facebook, etc... probably don't take WAI-ARIA's presentation role into account.

Would be nice if we could just do:
<a href="//www.mediawiki.org/" aria-label="Powered by MediaWiki"><img role="presentation" src=".../common/images/poweredby_mediawiki_88x31.png" alt="Powered by MediaWiki" width="88" height="31" /></a>
Comment 81 Wes Turner 2014-01-19 05:22:48 UTC
(In reply to comment #79)
> (In reply to comment #77)
> > This test fixture seems to indicate that a
> > 
> >      'background-image: url(' . $wgFooterIcons['powererdby']['src'] . ')';
> > 
> > CSS attribute containing input from configuration on the filesystem may be
> > stripped:
> > 
> > https://git.wikimedia.org/blob/mediawiki%2Fcore/
> > 6a2d25eed09c311c70657789b3f7a841bc5363db/
> > tests%2Fphpunit%2Fincludes%2FSanitizerTest.php#L253
> 
> Yes, all url() constructs will make checkCss consider the css unsafe. So
> you'll
> have to sanitize the background-image separate from the rest of the css.
> 
> > HTML::element states:
> > 
> >             // There's no point in escaping quotes, >, etc. in the contents
> > of
> >             // elements.
> > 
> > In this case, it is probably good to escape a '"; and/or javascript: in the
> > configuration-supplied variable.
> > 
> > It would be helpful if someone more familiar with the codebase could indicate
> > if there is a more appropriate function than htmlspecialchars (with
> > ENT_NOQUOTES/ENT_QUOTES) for this.
> 
> Escaping of quotes ('") are already handled by Html::element, ignore
> htmlspecialchars completely here.
> 
> For sanitizing the URL is safe/not javascript: use preg_match( '/^(' .
> wfUrlProtocols() . ')[^\s]+$/', ... ) to test if the protocol is whitelisted.

It look like Sanitizer::safeEncodeAttribute [would] also reference wfUrlProtocols, though it seems to only escape double single quotes ("''", "&#39;&#39"), expecting the input to have already been run through encodeAttribute and htmlspecialchars with ENT_QUOTES.

The documentation for http://php.net/manual/en/function.htmlspecialchars.php reads:


    * '"' (double quote) becomes '&quot;' when ENT_NOQUOTES is not set.
    * "'" (single quote) becomes '&#039;' (or &apos;) only when ENT_QUOTES is set.

And:

    ENT_QUOTES	Will convert both double and single quotes.
Comment 82 Wes Turner 2014-01-19 05:36:33 UTC
(In reply to comment #81)
> (In reply to comment #79)
> > (In reply to comment #77)
> > > This test fixture seems to indicate that a
> > > 
> > >      'background-image: url(' . $wgFooterIcons['powererdby']['src'] . ')';
> > > 
> > > CSS attribute containing input from configuration on the filesystem may be
> > > stripped:
> > > 
> > > https://git.wikimedia.org/blob/mediawiki%2Fcore/
> > > 6a2d25eed09c311c70657789b3f7a841bc5363db/
> > > tests%2Fphpunit%2Fincludes%2FSanitizerTest.php#L253
> > 
> > Yes, all url() constructs will make checkCss consider the css unsafe. So
> > you'll
> > have to sanitize the background-image separate from the rest of the css.
> > 
> > > HTML::element states:
> > > 
> > >             // There's no point in escaping quotes, >, etc. in the contents
> > > of
> > >             // elements.
> > > 
> > > In this case, it is probably good to escape a '"; and/or javascript: in the
> > > configuration-supplied variable.
> > > 
> > > It would be helpful if someone more familiar with the codebase could indicate
> > > if there is a more appropriate function than htmlspecialchars (with
> > > ENT_NOQUOTES/ENT_QUOTES) for this.
> > 
> > Escaping of quotes ('") are already handled by Html::element, ignore
> > htmlspecialchars completely here.
> > 
> > For sanitizing the URL is safe/not javascript: use preg_match( '/^(' .
> > wfUrlProtocols() . ')[^\s]+$/', ... ) to test if the protocol is whitelisted.
> 
> It look like Sanitizer::safeEncodeAttribute [would] also reference
> wfUrlProtocols, though it seems to only escape double single quotes ("''",
> "&#39;&#39"), expecting the input to have already been run through
> encodeAttribute and htmlspecialchars with ENT_QUOTES.
> 
> The documentation for http://php.net/manual/en/function.htmlspecialchars.php
> reads:
> 
> 
>     * '"' (double quote) becomes '&quot;' when ENT_NOQUOTES is not set.
>     * "'" (single quote) becomes '&#039;' (or &apos;) only when ENT_QUOTES is
> set.
> 
> And:
> 
>     ENT_QUOTES    Will convert both double and single quotes.

Updated this patch for this issue. Please advise if there is a local function more suitable than Html::safeEncodeAttribute.

https://gerrit.wikimedia.org/r/#/c/108221/
Comment 83 Gerrit Notification Bot 2014-01-19 05:37:00 UTC
Change 108221 had a related patch set uploaded by Jforrester:
Updated includes/Skin.php footer images #30113

https://gerrit.wikimedia.org/r/108221
Comment 84 Subfader 2014-01-19 14:13:47 UTC
My current header (MW 1.16)

<head>
<meta charset="utf-8" />
<meta property="og:site_name" content="My Wiki &minus; With Subtitle"/>
<meta property="og:type" content="Website"/>
<meta property="og:url" content="Article URL > same as link rel canonical"/>
<meta property="og:title" content="The Article Name"/>
<meta property="og:image" content="http://path.to/my/static/Logo.png (forced on every content-ns page)"/>
<meta property="og:image" content="/wiki/thumb.php?w=350&amp;f=Filename_of_article_image_1 (if given)" />
<meta property="og:image" content="/wiki/thumb.php?w=350&amp;f=Filename_of_article_image_2 (if given)" />
<meta property="og:description" content="Foo Bar" />
<meta name="description" content="Foo Bar" />
link rels
css
scripts
</head>
Comment 85 Jared Zimmerman (WMF) 2014-01-21 22:24:29 UTC
Thanks everyone for all the fast work to get this done, very excited to see it working.
Comment 86 Jared Zimmerman (WMF) 2014-03-01 06:55:46 UTC
Is this stalled again?

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links