Last modified: 2012-11-30 21:38:35 UTC
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.
bug 11456 related
Created attachment 4352 [details] adding [[Special:Mystyle]] and [[Special:Myscript]]
* Patch needs to use getSkinName() to get skin names, not nonexistent getName() * Doesn't seem to work at all for Monobook, this needs to be fixed * Patch doesn't apply cleanly to trunk; are you using an up-to-date copy of the software?
*** Bug 12485 has been marked as a duplicate of this bug. ***
(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.
*** Bug 15822 has been marked as a duplicate of this bug. ***
Created attachment 5384 [details] add monobook as a special case Adding monobook as a special case works, but the underlying issue (comment 5) should probably be looked at.
Created attachment 5385 [details] re-try
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.
Needs review. Kind, loving review. I'm willing to fix this if I can, so I've assigned it to myself I guess.
Created attachment 5391 [details] 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.
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.
(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.
*** Bug 22971 has been marked as a duplicate of this bug. ***
Isn't this now "resolved" by the [[Special:Mypage/skin.css]] and [[Special:Mypage/skin.jss]] functionality?
(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.
(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.
Is this still relevant with common.css/.js around?
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]].
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.
Created attachment 9990 [details] 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
Created attachment 9991 [details] 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.
(In reply to comment #28) > Created attachment 9991 [details] > 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. getRedirect()- just ignore the $wgUser global variable(its not used anywhere in this function)
Comment on attachment 5391 [details] adds getSkinName() to remove the special case Patch reviewed; Akshay's patch, attachment 9991 [details], obsoletes this.
Comment on attachment 9990 [details] It adds the functionality of Special:Myscript and Special:Mystyle Patch obsoleted by attachment 9991 [details], 3 minutes later. :-)
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: * marked the other patches on this bug "obsolete" (click Details, then Edit Details, then check the obsolete button and enter an explanation in the comments box) * changed the "reviewed" keyword to "need-review" 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.
(In reply to comment #33) > As such, suggesting WONTFIX. Okay.
*** Bug 42589 has been marked as a duplicate of this bug. ***
