Page MenuHomePhabricator

Change getPoweredBy to be customizable
Closed, ResolvedPublic

Description

Author: dasch

Description:
Would be nice if the function getPoweredBy could be changed to recive an array with picturepath, linkpath and alttext, so that someone can configure true LocalSettings if there is anything displayed or if there are more buttons shown.

Extensions could also use this to show a button there.


Version: 1.16.x
Severity: enhancement

Details

Reference
bz22463

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 21 2014, 10:56 PM
bzimport set Reference to bz22463.
bzimport added a subscriber: Unknown Object (MLST).

dasch wrote:

Possible Solution

Maybe this way, works for me

attachment Skin.diff ignored as obsolete

dasch wrote:

well maybe $wgPoweredBy should be set in default settings, than it would be easier to just add a button

dasch wrote:

Fixed some errors

Fixed an error I made and move the default powered by mediawiki to default settings, so that buttons could simply be added

attachment Skin.diff ignored as obsolete

The patch seems to hardcode $wgStylePath into the url for the image, which it probably shouldn't.

changed component to user interface.

It completely must not. It should honor $wgStylePath (not expanded until Setup.php), not hardcode the wiki url.

dasch wrote:

New Patch with value in Setup.php

Okay, I move it from DefaultSetting.php to Setup.php and used StylePath

attachment Skin.diff ignored as obsolete

  1. $wgPoweredBy should be initialized as an empty array, then the default value for MediaWiki can be added. Also needs documentation. This should be done in DefaultSettings, not Setup. Rather than putting $wgStylePath in the image path, just start with 'common/images...' (and then use $wgStylePath when generating the URL). As long as it's clearly documented that $wgPoweredBy images are relative to $wgStylePath, this is fine.
  1. Need a space after 'by' in the alt text.
  1. I would suggest using named array keys for the URL and image parameters for clarity. Something like:

array( 'url' => 'http://mediawiki.org', 'image' => 'common/images/poweredby_mediawiki_88x31.png' )

dasch wrote:

why does it have to be relativ to $wgStylePath?
I have
$wgPoweredBy['Semantic MediaWiki'] = array ("/extensions/SemanticMediaWiki/skins/images/smw_button.png","http://semantic-mediawiki.org/");
in my config, for example

well with the parameters for clarity I don't know how to make the foreach I have no experiance in programming, so I will not provide more patches, take it or leave it

(In reply to comment #8)

why does it have to be relativ to $wgStylePath?
I have
$wgPoweredBy['Semantic MediaWiki'] = array
("/extensions/SemanticMediaWiki/skins/images/smw_button.png","http://semantic-mediawiki.org/");
in my config, for example

That's easily done with '../extensions/SemanticMediaWiki/skins/images/smw_button.png'

well with the parameters for clarity I don't know how to make the foreach I
have no experiance in programming, so I will not provide more patches, take it
or leave it

Instead of using [0] and [1] to get the values, you'd use ['url'] and ['image']. The foreach wouldn't change.

(In reply to comment #8)

why does it have to be relativ to $wgStylePath?

Because the path to the skins is stored there. The user can change it. For example, Wikipedia has $wgStylePath="http://bits.wikimedia.org/skins-1.5"; and the icon is at http://bits.wikimedia.org/skins-1.5/common/images/poweredby_mediawiki_88x31.png

Your patch doesn't allow it. Moreover, it forces the user to have the wiki installed in the root. It would also break for a wiki placed on www.example.com/mediawiki/index.php.

dasch wrote:

This way?

okay, but with setting the default value I have one problem, this should work
this way.

When I set

$wgPoweredBy['Semantic MediaWiki'] = array(
'image' => "../extensions/SemanticMediaWiki/skins/images/smw_button.png",
'url' => "http://semantic-mediawiki.org/");

In LocalSettings I append it

When I set

$wgPoweredBy = array('Semantic MediaWiki' => array(
'image' => "../extensions/SemanticMediaWiki/skins/images/smw_button.png",
'url' => "http://semantic-mediawiki.org/"));

I will only see this one, so I think it should only be det in default settings
with the default value

So I put the default value in LocalSettings and put the StylePath into the function again

attachment Skin.diff ignored as obsolete

That looks better. It's probably worth to also htmlspecialchars $key

(In reply to comment #10)

(In reply to comment #8)

why does it have to be relativ to $wgStylePath?

Because the path to the skins is stored there. The user can change it. For
example, Wikipedia has $wgStylePath="http://bits.wikimedia.org/skins-1.5"; and
the icon is at
http://bits.wikimedia.org/skins-1.5/common/images/poweredby_mediawiki_88x31.png

Your patch doesn't allow it. Moreover, it forces the user to have the wiki
installed in the root. It would also break for a wiki placed on
www.example.com/mediawiki/index.php.

I really don't like the idea of this being required as relative to $wgStylePath. It should be absolute the same way $wgLogo, $wgFavicon, etc... all are. There are reasons to want to make this relative to the extensions resources, style path, or even the upload path, what it is relative to should NOT be dictated by the system. And the idea of using ../extensions/ or ../images/ is messed up, that's making assumptions about where the user's extensions and images may be. Upload paths, and more recently extension resource paths are customizable.

Created attachment 7806
Patch to add new SkinGetPoweredBy hook to getPoweredBy()

Well, it looks like I no longer have commit access for MediaWiki, I guess as a result of separate permissions having been created for core and for the extensions. So attached is the code I wanted to check in, that I think solves this problem in a logical way, using a hook. Assuming this looks alright, it would be great if someone could check this in.

Thanks,
Yaron

attachment SkinGetPoweredBy hook.patch ignored as obsolete

Just wondering, do we really want every extension adding buttons there? The average wiki (or wikipedia, which i geuss isn't that average) has something like 57 extensions installed. If each one added a button it'd get might crowded down there. On the other hand, I suppose (in the ideal world where extension authors show restraint) only extensions that significantly change mediawiki (like smw) would add a button.

Yaron's patch applied with r76434

(In reply to comment #15)

Just wondering, do we really want every extension adding buttons there? The
average wiki (or wikipedia, which i geuss isn't that average) has something
like 57 extensions installed. If each one added a button it'd get might crowded
down there. On the other hand, I suppose (in the ideal world where extension
authors show restraint) only extensions that significantly change mediawiki
(like smw) would add a button.

True, however I suppose its more for things like SMW, wiki farms and buttons for hosting providers. For example: http://shoutwiki.com and Sourceforge project web sites.

(In reply to comment #17)

(In reply to comment #15)

Just wondering, do we really want every extension adding buttons there? The
average wiki (or wikipedia, which i geuss isn't that average) has something
like 57 extensions installed. If each one added a button it'd get might crowded
down there. On the other hand, I suppose (in the ideal world where extension
authors show restraint) only extensions that significantly change mediawiki
(like smw) would add a button.

True, however I suppose its more for things like SMW, wiki farms and buttons
for hosting providers. For example: http://shoutwiki.com and Sourceforge
project web sites.

True, though don't most of the people currently doing buttons for those in a custom way try to add new buttons rather than replace the MediaWiki powered by?

Shouldn't we also try standardizing a newer button list (copyright, powered by, hosted by, extension powered [just for smw like large exts], etc...) which is supported by skins that have specifically coded support for it and can support more than just two buttons?

dasch wrote:

I'm really disapoinnted about what was made from my idea. My idea was to give the possibilty to add buttons through a variable so that it could also easily be done through configuration without any extension. With the hook implemented now it's just more complex. The point was that maybe somebody would also like to add a button for his hoster or something like this. I really do not understand why my simple solution was not applied but a hook was added that fast.

dasch wrote:

alternative idea/patch

Maybe it should be simple done this way. Then it would also be possible to add icons from other pages.
Like this
$wgPoweredBy['BinLayer'] = array (
'image' => "http://bl.wavecdn.de/lan/banner/de/88_31_1.gif",
'url' => "https://binlayer.com/ref-191183.html");

Just don't know now where to put the standard mediawiki button. Must be somewhere before local settings so it could be turned of and somewhere after Setup.php so that the skin path is already set

attachment skin.patch ignored as obsolete

(In reply to comment #19)

I'm really disapoinnted about what was made from my idea. My idea was to give
the possibilty to add buttons through a variable so that it could also easily
be done through configuration without any extension. With the hook implemented
now it's just more complex. The point was that maybe somebody would also like
to add a button for his hoster or something like this. I really do not
understand why my simple solution was not applied but a hook was added that
fast.

It's very easy now too. Please see what I added to LocalSettings.php of Translatewiki:

$wgHooks['SkinGetPoweredBy'][] = 'efPoweredBy';
function efPoweredBy( &$text ) {
$text = "<div class='mw_poweredby'><a href=\"http://www.netcup.de/\" title=\"Powered by netcup - Webspace and vServer\" target=\"_blank\">Powered by netcup - Webspace and vServer</a></div>";
return true;
}

dasch wrote:

well... but that's truely no enduser solution and it does not give a standarized output. With this hook you can and nearly anything to skin cause you give the full HTML. When somebody uses a different skin this may even lead to total page brake. That's not a nice easy to use solution. The idea is that every admin can add an icon just by putting in the url of the picture and a link. An that the output is that reliable that every skin designer can use the given format und fit it into the skin. In some skins this means that they are in one row in other skins this maybe in a colon.

I like DaSch approach. It could be an array with either or image and url or html.

I don't see a clean way to merge that with Modern approach of showing just text, though.

dasch wrote:

I think the best would be if there where 3 thinks in the array
image, url, title

If image and url are given, the image is shown, if no image is given then it would be a plain link with title as text

dasch wrote:

For plain-links and image links

This is an alternative that switches between image-links and plain-link by checking if an image is given or not. There is no HTML included so designers can really on the HTML that is returned.

attachment skin.patch ignored as obsolete

dasch wrote:

reworked new patch

rework with some fixes
now even works with only a key and an url given

attachment skin.patch ignored as obsolete

I'm playing around with footerlinks right now a side effect of my tweaks would actually allow for a SkinTemplateOutputPageBeforeExec hook to $tpl->set('poweredbysmwico', '...'); and $tpl->data["footerlinks"]["icons"]["right"][] = "poweredbysmwico"; to add another icon. This flexibly also permits things on the left, and works nice in vector.
Overcrowding in monobook also has a preference to allow the icons to be trimmed back to just a normal 1 on each side.

Actually thinking about the SkinGetPoweredBy hook, it actually doesn't add any special new functionality. Using SkinTemplateOutputPageBeforeExec you could already use $tpl->set('poweredbyico', to replace the ico in the same way that the new hook allows. It's a little redundant and doesn't add enough flexibility.

I ended up dropping that part out of the footerlinks code and instead implementing $wgFooterIcons in r77741.

dasch wrote:

Well, somehow we now ended up with a solution that is far away from what I though of. Simply adding

$wgFooterIcons['poweredby']['BinLayer'] = array (
'src' => "http://bl.wavecdn.de/lan/banner/de/88_31_1.gif",
'url' => "https://binlayer.com/ref-191183.html",
'alt' => "BinLayer");

to LocalSettings.php does not work and that was the intention. Like I see I would have to create a Extension and using this Hook for adding an Icon. The idea was that this could be used by users and developers, not only by developers

(In reply to comment #29)

Well, somehow we now ended up with a solution that is far away from what I
though of. Simply adding

$wgFooterIcons['poweredby']['BinLayer'] = array (
'src' => "http://bl.wavecdn.de/lan/banner/de/88_31_1.gif",
'url' => "https://binlayer.com/ref-191183.html",
'alt' => "BinLayer");

to LocalSettings.php does not work and that was the intention. Like I see I
would have to create a Extension and using this Hook for adding an Icon. The
idea was that this could be used by users and developers, not only by
developers

Erm, you might want to double check some stuff, that IS supposed to work. And my tests with stuff like that wave worked perfectly fine.

dasch wrote:

well, the implementation is buggy, the icons added by mediawiki and semantic mediawiki show of with $this->html('poweredbyico') but the one added through $wgFooterIcons does not show up

poweredbyico and copyrightico are deprecated.

dasch wrote:

Maybe you should take a look at the Skin I'm maintaining and what your Deprecation does to my code
https://sourceforge.net/apps/trac/wecowi/browser/trunk/skins/Cavendish.php

dasch wrote:

And now again something was changed and my costum icon is not displayed anymore... what's happening here :o

sumanah wrote:

Comment on attachment 7806
Patch to add new SkinGetPoweredBy hook to getPoweredBy()

Marked obsolete since patch has already been applied.

sumanah wrote:

Comment on attachment 7820
reworked new patch

Marked as obsolete since patch no longer applies cleanly to Subversion trunk, per Rusty Burchfield's automated testing https://docs.google.com/spreadsheet/ccc?key=0Ah_71HHl7qa7dGtvSms3TGpHQU9NU2Y1VmNzUEUteWc

sumanah wrote:

Dasch, thank you for the patch. I'm removing the need-review and patch keywords since the MediaWiki codebase has changed so much since you submitted the patch that the patch no longer applies cleanly to Subversion trunk. If you're still interested in this issue, maybe you could come into the MediaWiki-General IRC channel and discuss the situation and possible approaches before revising. Thanks!

(In reply to comment #0)

Would be nice if the function getPoweredBy could be changed to recive an
array
with picturepath, linkpath and alttext, so that someone can configure true
LocalSettings if there is anything displayed or if there are more buttons
shown.

Extensions could also use this to show a button there.

This is today provided by https://www.mediawiki.org/wiki/Manual:$wgFooterIcons

Resolving as WORKSFORME.

If this doesn't work in your website then please file a bug report specific to your case. Thank you!

This was fixed rather than just working so marking it as FIXED instead of WORKSFORME.

Also adding bug 46807 to deal with the fact that the old hook and *ico keys are still around and don't support wgFooterIcons.