| • bzimport | |
| Sep 18 2006, 1:10 AM |
| F3240: skin.diff | |
| Nov 21 2014, 9:25 PM |
Author: dto
Description:
Several functions in the Skin class are marked with /*static*/. These functions,
however, are not actually static, and some of them will even result in errors if
actually called statically.
Version: 1.8.x
Severity: enhancement
dto wrote:
Changing summary -- in addition to the functions in the Skin class marked
/*static*/, _all_ of Linker's methods can and should be static, as they are all
utility methods and should not need an instance of Linker/Skin to be called.
dto wrote:
patch
Patch for both Skin and Linker. Makes /*static*/ Skin methods static and makes
all of Linker's methods static.
attachment skin.diff ignored as obsolete
rotemliss wrote:
Well, there is a difference. In Skin, they are already static, and are just not
marked as such, and are called statically from most places (should be fixed in
some places). In Linker, they are *not* static, and are never called statically.
I agree that the Linker methods should be static, and we should not use the Skin
object for them, but it's another bug, and is a bit harder to fix.
dto wrote:
(In reply to comment #4)
Well, there is a difference. In Skin, they are already static, and are just not
marked as such, and are called statically from most places (should be fixed in
some places).
Which functions are being talked about? Skin::makeNSUrl, for example, currently
refers to $this-> (instead of self::) and will, I think, result in an error if
called statically.
I agree that the Linker methods should be static, and we should not use the Skin
object for them, but it's another bug, and is a bit harder to fix.
Yeah. My last patch could probably be a start, because it doesn't break any
behavior, as far as I can tell. $wgUser->getSkin()->[Linker function] will still
work.
rotemliss wrote:
(In reply to comment #5)
(In reply to comment #4)
Well, there is a difference. In Skin, they are already static, and are just not
marked as such, and are called statically from most places (should be fixed in
some places).Which functions are being talked about? Skin::makeNSUrl, for example, currently
refers to $this-> (instead of self::) and will, I think, result in an error if
called statically.
Calling a static function as $foo->bar() will not result an error, but a notice.
The ability to define functions as "static" is new in PHP 5, and before that you
could call *any* function as both static (Foo::bar) and non-static
($foo->bar()). Now you should define the static functions, but because they are
still not defined as static and it is not *required* to call them as static,
people sometimes call them in the non-static form ($foo->bar()), and these
places should be fixed to Foo::bar. I also think that we have to set them to
static, but there are some places where you didn't change $foo->bar() to
Foo::bar in your patch. I will fix these calls and set the functions as static
as early as I have some time.
I agree that the Linker methods should be static, and we should not use the Skin
object for them, but it's another bug, and is a bit harder to fix.Yeah. My last patch could probably be a start, because it doesn't break any
behavior, as far as I can tell. $wgUser->getSkin()->[Linker function] will still
work.
Calling static functions as $foo->bar() may work now, but it's not right and may
not work in future PHP versions. If you want to set the Linker functions as
static, you (or someone else) have to change all the calls for them.
rotemliss wrote:
Applied the first patch of the patch to r16601, along with changes in all the
references to the functions. Please open another bug for the Linker functions.
dto wrote:
(In reply to comment #7)
Applied the first patch of the patch to r16601, along with changes in all the
references to the functions.
Thanks.
Please open another bug for the Linker functions.
bug 7405