| Danny_B | |
| Aug 3 2006, 1:32 AM |
| F3119: fix.patch | |
| Nov 21 2014, 9:22 PM |
Please add Special:Mystyle and Special:Myscript as complements to Special:Mypage
and Special:Mytalk.
Although simple using of Special:Mypage/monobook.css or
Special:Mypage/monobook.js is possible at the moment, prosposed special pages
should reflect on user's actual skin.
Therefore simple general linking to user's style/script instead of list of links
by skin would be possible.
Thanks
Danny B.
Version: unspecified
Severity: enhancement
adding [[Special:Mystyle]] and [[Special:Myscript]]
attachment SpecialPage.php.patch ignored as obsolete
ayg wrote:
ayg wrote:
(In reply to comment #3)
- Doesn't seem to work at all for Monobook, this needs to be fixed
This is because Monobook doesn't initialize the skinname, stylename, or template member variables until SkinMonoBook::initPage() is called. Someone needs to either use a different way than getSkinName() to do this, or figure out if it's possible to safely move the skinname initialization in MonoBook.php to happen at construction time.
mike.lifeguard+bugs wrote:
add monobook as a special case
Adding monobook as a special case works, but the underlying issue (comment 5) should probably be looked at.
attachment 1.diff ignored as obsolete
ermwiki wrote:
I think there is a problem.
The default skin is Monobook, but I think we can change it. If we will change it in a web, then we need to write if "$wgUser->getSkin()->getSkinName()" is null, so the computer need to return the title to the CSS/JS page of the default skin in the web.
mike.lifeguard+bugs wrote:
Needs review. Kind, loving review.
I'm willing to fix this if I can, so I've assigned it to myself I guess.
mike.lifeguard+bugs wrote:
adds getSkinName() to remove the special case
A few skins didn't have getSkinName(), so this adds it where it doesn't exist already. Should fix this for the standard skins no matter what skin you're using.
attachment MyScript and MyStyle.patch ignored as obsolete
ayg wrote:
First some specific comments (mostly nitpicks, except the second point):
'Mytalk' => array( 'SpecialMytalk' ),+ 'Myscript' => array( 'SpecialMyscript' ),
+ 'Mystyle' => array( 'SpecialMystyle' ),'Mycontributions' => array( 'SpecialMycontributions' ),
Spaces are being used for indentation here, not tabs, so that it aligns correctly regardless of tab width. That should be kept for the new entries.
+ return Title::makeTitle( NS_USER, $wgUser->getName() . '/' . $wgUser->getSkin()->getSkinName() . '.css' );
...
+ return Title::makeTitle( NS_USER, $wgUser->getName() . '/' . $wgUser->getSkin()->getSkinName() . '.js');
Should this be makeTitleSafe, with error handling? What happens if the username plus skin name add up to more than 251 bytes or so? What if the skin name contains crazy characters?
class SkinChick extends SkinTemplate {
+
+ function getSkinName() {
No need for the blank line here (or for any of the others).
+ return "chick";
For strings where no interpolation is needed, single quotes are better style (and for all the others).
More general issue: this doesn't work for custom skins. If they inherit from MonoBook, for instance, they would direct to monobook.css, which is wrong. The name should come from whatever is used by the bit of code that loads user CSS and JS. In fact, Skin::getSkinName() appears to be the function that's used, so whatever you're doing looks wrong. The problem is that the $skinname member variable is for some reason not initialized at this point. You should figure out why that is, initialize it, and then use getSkinName() without trying to manually override it and duplicate the info that's already set in $skin->skinname.
mike.lifeguard+bugs wrote:
(In reply to comment #12)
First some specific comments (mostly nitpicks, except the second point):
'Mytalk' => array( 'SpecialMytalk' ),+ 'Myscript' => array( 'SpecialMyscript' ),
+ 'Mystyle' => array( 'SpecialMystyle' ),'Mycontributions' => array( 'SpecialMycontributions' ),Spaces are being used for indentation here, not tabs, so that it aligns
correctly regardless of tab width. That should be kept for the new entries.+ return Title::makeTitle( NS_USER, $wgUser->getName() . '/' . $wgUser->getSkin()->getSkinName() . '.css' );
...
+ return Title::makeTitle( NS_USER, $wgUser->getName() . '/' . $wgUser->getSkin()->getSkinName() . '.js');Should this be makeTitleSafe, with error handling? What happens if the
username plus skin name add up to more than 251 bytes or so? What if the skin
name contains crazy characters?class SkinChick extends SkinTemplate {
+
+ function getSkinName() {No need for the blank line here (or for any of the others).
+ return "chick";
For strings where no interpolation is needed, single quotes are better style
(and for all the others).More general issue: this doesn't work for custom skins. If they inherit from
MonoBook, for instance, they would direct to monobook.css, which is wrong. The
name should come from whatever is used by the bit of code that loads user CSS
and JS. In fact, Skin::getSkinName() appears to be the function that's used,
so whatever you're doing looks wrong. The problem is that the $skinname member
variable is for some reason not initialized at this point. You should figure
out why that is, initialize it, and then use getSkinName() without trying to
manually override it and duplicate the info that's already set in
$skin->skinname.
I totally do not understand what you are saying here. I think this is basically beyond my abilities, so I'm removing myself from assigned.
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and [[Special:Mypage/skin.jss]] functionality?
mike.lifeguard+bugs wrote:
(In reply to comment #15)
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and
[[Special:Mypage/skin.jss]] functionality?
Obviously not - read comment 0.
mike.lifeguard+bugs wrote:
(In reply to comment #15)
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and
[[Special:Mypage/skin.jss]] functionality?
Sorry, that's actually true (though I do wonder what happens if you have a subpage called skin.css...)
(In reply to comment #17)
(In reply to comment #15)
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and
[[Special:Mypage/skin.jss]] functionality?Sorry, that's actually true (though I do wonder what happens if you have a
subpage called skin.css...)
Thats an enwikipedia-only js hack, not a fix for the bug. (And to answer your question, the redirect does not happen if you have a subpage named /skin.js ( resp .css)).
However with that said, you can do special:mypage/common.css (resp. js) as user:Myname/common.css (resp. .js) is loaded for all skins (as of 1.17), which in my mind is a fix to this bug.
(In reply to comment #18)
(In reply to comment #17)
(In reply to comment #15)
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and
[[Special:Mypage/skin.jss]] functionality?Sorry, that's actually true (though I do wonder what happens if you have a
subpage called skin.css...)Thats an enwikipedia-only js hack, not a fix for the bug. (And to answer your
question, the redirect does not happen if you have a subpage named /skin.js (
resp .css)).However with that said, you can do special:mypage/common.css (resp. js) as
user:Myname/common.css (resp. .js) is loaded for all skins (as of 1.17), which
in my mind is a fix to this bug.
It is not fix. It is just a workaround. And it actually doesn't solve the requested thing. Reopening.
I'd say no, since some version ago subpages and several parameters are preserved, the following works:
[[Special:Mypage/common.js]]
[[Special:Mypage/common.css]]
(as does any other subpage for that matter, such as linking to a specific skin)
(In reply to comment #20)
Is this still relevant with common.css/.js around?
Yes. The opening comment was about a 301 redirect from Special:Myscript to the user's active user script subpage and a 301 redirect from Special:Myskin to the user's active user skin subpage. Neither of these have been implemented.
Using common.js/common.css as a workaround feels like a hack and has been rejected by the opening poster already (comment #19). Re-opening.
(In reply to comment #22)
(In reply to comment #20)
Is this still relevant with common.css/.js around?
Yes. The opening comment was about a 301 redirect from Special:Myscript to the
user's active user script subpage and a 301 redirect from Special:Myskin to the
user's active user skin subpage. Neither of these have been implemented.Using common.js/common.css as a workaround feels like a hack and has been
rejected by the opening poster already (comment #19). Re-opening.
In that case can someone provide a use case for when you'd want to be able to write generic instructions for adding stuff to the personal js of the skin you are on, no matter what skin it is, but not have that css/js be applied to all skins.
The original use case of - To add userscript foo to your account go to [[special:myscript]] and add line importScript("whatever") - doesn't apply anymore since any such instructions would want to use [[special:mypage/common.js]].
(In reply to comment #23)
The original use case of - To add userscript foo to your account go to
[[special:myscript]] and add line importScript("whatever") - doesn't apply
anymore since any such instructions would want to use
[[special:mypage/common.js]].
Exactly, and if the script in question would be monobook specific (or whatever) it would link to [[Special:MyPage/monobook.js]].
sumanah wrote:
Adding the "reviewed" keyword since in comment 12 Aryeh reviewed the patch.
I'm lowering importance of this since 7 months have gone by without anyone presenting a usecase.
chughakshay16 wrote:
It adds the functionality of Special:Myscript and Special:Mystyle
here i am getting the value for the skin name from the global varibale $wgDefaultSkin which could be modified in LocalSettings.php
attachment specialPage.patch ignored as obsolete
chughakshay16 wrote:
It adds the functionality of Special:Myscript and Special:Mystyle
Compared to the patch which i submitted before this one checks for the user skin preferences , and with the current version of mediawiki it would work with custom skins as well(as all the skins are listed in preferences->appearance items)
so here instead of using $wgDefaultSkin which would have to be modified by the user in LocalSettings.php for to make a change in their skin it would be better to use
$this(SpecialPage object)->getUser()->getSkin()->getSkinName() as it takes care of the overridden skin parameters as well.
Attached:
chughakshay16 wrote:
(In reply to comment #28)
Created attachment 9991 [details]
It adds the functionality of Special:Myscript and Special:MystyleCompared to the patch which i submitted before this one checks for the user
skin preferences , and with the current version of mediawiki it would work with
custom skins as well(as all the skins are listed in preferences->appearance
items)so here instead of using $wgDefaultSkin which would have to be modified by the
user in LocalSettings.php for to make a change in their skin it would be better
to use
$this(SpecialPage object)->getUser()->getSkin()->getSkinName() as it takes care
of the overridden skin parameters as well.
getRedirect()- just ignore the $wgUser global variable(its not used anywhere in this function)
Attached:
sumanah wrote:
Comment on attachment 5391
adds getSkinName() to remove the special case
Patch reviewed; Akshay's patch, attachment 9991, obsoletes this.
sumanah wrote:
Comment on attachment 9990
It adds the functionality of Special:Myscript and Special:Mystyle
Patch obsoleted by attachment 9991, 3 minutes later. :-)
sumanah wrote:
Akshay, thanks for the patch. To indicate that your new patch needs review, and that it is now the best and newest patch you've suggested, I:
Otherwise, code reviewers get confused. So you can do this yourself, the next time you are updating a patch you've suggested, or adding a patch to a bug where there was previously a patch that's already been reviewed and rejected.
Thank you!
Repeating earlier thread with arguments:
(In reply to comment #24)
(In reply to comment #23)
The original use case of - To add userscript foo to your account go to
[[special:myscript]] and add line importScript("whatever") - doesn't apply
anymore since any such instructions would want to use
[[special:mypage/common.js]].Exactly, and if the script in question would be monobook specific (or whatever)
it would link to [[Special:MyPage/monobook.js]].
So what is the use case of linking to the script-page of the currently logged user's skin preference? (in addition to linking to a specific skin or the load-in-all skins script)
(In reply to comment #26)
I'm lowering importance of this since 7 months have gone by without anyone
presenting a usecase.
As such, suggesting WONTFIX.