| Ammarpad | |
| Sep 21 2020, 6:12 AM |
| F32360604: Screen Shot 2020-09-23 at 9.29.11 AM.png | |
| Sep 23 2020, 10:06 AM |
| F32360625: Screen Shot 2020-09-23 at 9.46.14 AM.png | |
| Sep 23 2020, 10:06 AM |
| F32360732: Screen Shot 2020-09-23 at 10.59.44 AM.png | |
| Sep 23 2020, 10:06 AM |
| F32360606: Screen Shot 2020-09-23 at 9.30.08 AM.png | |
| Sep 23 2020, 10:06 AM |
| F32358363: Screen Shot 2020-09-21 at 2.40.19 PM.png | |
| Sep 21 2020, 9:40 PM |
| F32358353: Screen Shot 2020-09-21 at 2.35.34 PM.png | |
| Sep 21 2020, 9:39 PM |
| F32358360: Screen Shot 2020-09-21 at 2.36.43 PM.png | |
| Sep 21 2020, 9:39 PM |
| F32358347: Screen Shot 2020-09-21 at 10.26.37 PM.png | |
| Sep 21 2020, 9:35 PM |
There are some unnecessary hacks in Vector and MobileFrontend for this extension and these should be cleaned up as they cannot be sustained in perpetuity.
It's the responsibility of the extension to make sure it looks good and works correctly with the existing infrastructure, just like how every other extension does.
"Unnecessary" in what sense?
Yes, ideally certain features would be supported in core and implemented by all skins, but that's not always practical.
Change 628740 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Vector@master] Remove code that does not belong to Vector skin
The hack's comment says "first item in the personal menus", and that's how I see it in my local. And that's how it is currently in production, Wikimedia Commons: https://commons.wikimedia.org/wiki/Main_Page or MetaWiki: https://meta.wikimedia.org/wiki/Main_Page
Before your gerrit patch:
Is this image from your local? It's different from what's in production.
After gerrit patch:
Yes, and that's how production looks currently. That means no change, and the patch works correctly.
Sorry my screenshot was confusing. Let me try that again.
When logged out Vector injects a menu item #pt-anonuserpage at the front. This is a hack on Vector's part and should be done in core, but is currently done in Vector.
This should always appear in front of ULS like so:
It's my understanding that ULS hides this in our production configurations, however with a very visible flash of unstyled content (which is a bug BTW and should be fixed).
You can however replicate this incognito window with https://commons.wikimedia.org/wiki/Main_Page?safemode=1
After your change, ULS moves to after the #pt-anonuserpage element.
When logged out Vector injects a menu item #pt-anonuserpage at the front. This is a hack on Vector's part and should be done in core, but is currently done in Vector.
Thanks. I see what you mean. That needs fixing itself. I will attempt that
After your change, ULS moves to after the #pt-anonuserpage element.
Yes, of course that's because the anon placeholder is a hack string concatenation that's coming after ULS already claims the first spot via array union. Once the placeholder item becomes integrated menu data item, the ULS will get what it wants.
Change 629324 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Provide basic anon personal menu placeholder.
After moving the placeholder to core, ULS will get what it wants. The patch will also allow removal of the placeholder duplication in Monobook as well as removal of multiple hacks for the extension in both Vector and MonoBook as well as another skin that seems to be replicating the same thing.
| Default basic placeholder by core | Vector styles it | MonoBook styles it | Timeless unsets it** |
 |  |  |  |
Note: Timeless did unsets it in this image otherwise duplicate 'Not logged in' text will appear in between ULS extension trigger and Talk link.
Thanks for working on this! We will need patches for Vector and Monobook removing this code to avoid duplicating the menu item.
I think it's fine to ship this to Modern.
Change 629457 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/Timeless@master] Timeless: Unset anon placeholder item
Change 629465 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/skins/MonoBook@master] Remove ULS extension and anon placeholder hacks.
Change 629457 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Timeless: Unset anon placeholder item
Change 628740 merged by jenkins-bot:
[mediawiki/skins/Vector@master] Remove ULS extension hack and anon placeholder code
Change 629465 merged by jenkins-bot:
[mediawiki/skins/MonoBook@master] Remove ULS extension and anon placeholder hacks.
Change 629324 merged by jenkins-bot:
[mediawiki/core@master] Provide basic anon personal menu placeholder.
@Jdlrobson I'd need your help on how to reproduce the problem that $wgULSPosition ='none' hack is supposed to solve in MobileFrontend. Because me I am not seeing anything if I removed it (MobileFrontendHooks.php#L123).
For context, the hack was added in T59091 several years ago.
I believe this can be removed now.
It was originally added as part of https://phabricator.wikimedia.org/T59091 however now ULS specifically guards against Minerva since 4 months ago here:
https://github.com/wikimedia/mediawiki-extensions-UniversalLanguageSelector/blob/f13d28f4d1131931325bfde1d71e12d5dccd9550/includes/UniversalLanguageSelectorHooks.php#L122
Change 630720 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/extensions/MobileFrontend@master] Remove $wgULSPosition hack for ULS extension
Change 630720 merged by jenkins-bot:
[mediawiki/extensions/MobileFrontend@master] Remove $wgULSPosition hack for ULS extension
