Last modified: 2006-09-25 02:47:09 UTC

Wikimedia Bugzilla is closed!

Wikimedia has migrated from Bugzilla to Phabricator. Bug reports should be created and updated in Wikimedia Phabricator instead. Please create an account in Phabricator and add your Bugzilla email address to it.
Wikimedia Bugzilla is read-only. If you try to edit or create any bug report in Bugzilla you will be shown an intentional error message.
In order to access the Phabricator task corresponding to a Bugzilla report, just remove "static-" from its URL.
You could still run searches in Bugzilla or access your list of votes but bug reports will obviously not be up-to-date in Bugzilla.
Bug 7357 - Make supposedly static methods of Skin actually static
Make supposedly static methods of Skin actually static
Status: RESOLVED FIXED
Product: MediaWiki
Classification: Unclassified
General/Unknown (Other open bugs)
1.8.x
All All
: Normal enhancement with 1 vote (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks: code_quality
  Show dependency treegraph
 
Reported: 2006-09-18 01:10 UTC by Dan Li
Modified: 2006-09-25 02:47 UTC (History)
0 users

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


Attachments
patch (2.86 KB, patch)
2006-09-18 01:10 UTC, Dan Li
Details
patch (29.31 KB, patch)
2006-09-19 06:30 UTC, Dan Li
Details

Description Dan Li 2006-09-18 01:10:01 UTC
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.
Comment 1 Dan Li 2006-09-18 01:10:48 UTC
Created attachment 2368 [details]
patch

Simple fix.
Comment 2 Dan Li 2006-09-19 06:29:24 UTC
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.
Comment 3 Dan Li 2006-09-19 06:30:14 UTC
Created attachment 2371 [details]
patch

Patch for both Skin and Linker. Makes /*static*/ Skin methods static and makes
all of Linker's methods static.
Comment 4 Rotem Liss 2006-09-19 12:32:17 UTC
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.
Comment 5 Dan Li 2006-09-19 21:14:45 UTC
(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.
Comment 6 Rotem Liss 2006-09-20 09:41:31 UTC
(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.
Comment 7 Rotem Liss 2006-09-22 13:08:22 UTC
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.
Comment 8 Dan Li 2006-09-25 02:47:09 UTC
(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

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


Navigation
Links