| Nirmos | |
| Jun 5 2020, 5:29 AM |
| F31854325: UI messed up 20200605 (cropped).png | |
| Jun 5 2020, 5:29 AM |
#p-coll-print_export is now at the bottom. #p-lang used to be at the bottom. Is this intentional?
After the patch that made the TOOLBOX editable in the onSidebarBeforeOutput() hook the "Tools" and "Languages" portlets (sections) moved up in the order, above "In other projects" and "Print/Export" sections, which are created by extensions in the onSidebarBeforeOutput() hook.
Previous order:
#p-navigation #p-wikibase-otherprojects #p-coll-print_export #p-tb #p-lang
Current order:
#p-navigation #p-tb #p-lang #p-wikibase-otherprojects #p-coll-print_export
The sidebar order was changed today. See https://en.wikipedia.org/wiki/Wikipedia:Requests_for_comment/2020_left_sidebar_update . If you would prefer another implementation of that task, you will need to consult on your local wiki. I think this task should be rejected.
I think you have misidentified the cause of this. Unless there’s something really wrong with our processes, local discussion in English Wikipedia shouldn’t have affected order in other projects without consulting them first. This must be caused by some of refactoring in Vector happening recently, not by English Wikipedia discussion.
All of them that don’t have * coll-print_export specified in MediaWiki:Sidebar, as far as I am aware. ‘In other projects’ also seems to have changed its position, see for example https://sv.wikipedia.org/wiki/Pietro_Castelli
$sidebar['TOOLBOX'], $sidebar['LANGUAGES'] are added to the array before the
onSidebarBeforeOutput() hooks of Wikibase and ElectronPdf are run (these add #p-wikibase-otherprojects, #p-coll-print_export).
TOOLBOX and LANGUAGES used to be added by the skin after this method (buildSidebar()) is called.
EDIT: Related: T256071: Collection extension should be made aware of the ElectronPdf extension: Settle on a common sidebar key for both extensions
Interestingly moving the hook call above $sidebar['TOOLBOX'] would break CiteThisPage, MachineVision and ShortUrl or UrlShortener on wikis that didn't add TOOLBOX to MediaWiki:Sidebar.
EDIT: No, it does not. This is PHP, not a strict programming language... $sidebar['TOOLBOX'] is magically initialized with an empty array when $sidebar['TOOLBOX']['extensionentry'] = is added.
Adding the identifier equivalent of #p-wikibase-otherprojects and #p-coll-print_export (see below JdlRobson's comment) to the end of MediaWiki:Sidebar should restore the order though, I think.
You can control the order of these in [[MediaWiki:Sidebar]]
* navigation ** mainpage|mainpage-description ** Wikipedia:Contents|contents ** currentevents-url|currentevents ** randompage-url|randompage ** Wikipedia:About|aboutsite ** contact-url|contactpage ** sitesupport-url|sitesupport * SEARCH * wikibase-otherprojects * coll-print_export * interaction ** helppage|help ** portal-url|portal ** recentchanges-url|recentchanges ** Wikipedia:File Upload Wizard|upload
These don't seem to be explictly mentioned in https://en.wikipedia.org/wiki/MediaWiki:Sidebar
Let me know if that works and if so I'll send a user notice in case other projects are impacted. We have been making a few technical changes in this area to give the sidebar more flexibility.
The language portlet can be quite tall, and is the only one with interactive widgets attached (Wikidata, ULS). This is generally always expected to be last, and that seems both design/UX-wise and historically preferred.
The "MediaWiki:Sidebar" configuration exists to control the customisable content, generally the top two-thirds of the sidebar of localised links and such.
It generally never mentions the "Language" list, as that is controlled by core and the skin (and is also page dependent in plain MW core). The section "Print/Export" is also appended conditionally by core. The config page would most certainly would never mention extension-controlled additions.
If appendixes by extensions now go to after the Language list, that is a regression.
If core now appends "Print" after it appends "Languages", that is a regression.
I don't know if we need compat with the specific way things works, so long as the outcome is that in production and on new wikis by default the order is correct again.
This regression also affects all pages on the English Wikipedia.
I do not think we can require 900+ sidebars to be reconfigured, as well as every third-party's wikis sidebar, and every new MW install, and require every sidebar-related extension to now require end-users to reconfigure their sidebar.
Even then, also note that the sidebar is localised and that viewing the site in an alternate language (e.g. a Japanese user on English Wikipedia) will use the MW-default sidebar for Japanese and thus cannot be easily configured on-wiki even if that wasn't infeasible.
Change 602795 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Call onSidebarBeforeOutput() hook before populating TOOLBOX and LANGUAGES
Per the analysis in T254546#6196942 patch 602795 restores the order and keeps the refactored functionality too.
A note: extensions assume that $sidebar['TOOLBOX'] is set to an array. It seems to me prior to the refactoring patch this was only the case if TOOLBOX was added to MediaWiki:Sidebar, thus wikis without that (eg. https://sv.wikipedia.org/wiki/Pietro_Castelli) did not show the "Cite this page" link, silently ignoring the error in the extension's hook. However, I can't confirm this, I haven't found signs of error handling in the HookContainer, thus this assumption might turn out to be false. In that case this patch might remove the "Cite this page" link on those wikis without TOOLBOX in MediaWiki:Sidebar, causing regression.
svwiki, commons, wikidata:
Previous order: #p-wikibase-otherprojects, #p-coll-print_export BEFORE TOOLBOX (#p-tb), LANGUAGE (#p-lang)
Current order: TOOLBOX, LANGUAGE before #p-wikibase-otherprojects, #p-coll-print_export
enwiki:
Previous order: TOOLBOX before #p-wikibase-otherprojects, #p-coll-print_export before LANGUAGE
Current order: TOOLBOX, LANGUAGE before #p-wikibase-otherprojects, #p-coll-print_export
enwiki has TOOLBOX in MediaWiki:Sidebar, thus their placing of "Tools" was not affected, only "Languages".
Recently the community decided to change this and move "Print / export" above "Toolbox" (ref). This could be achieved by adding coll-print_export before TOOLBOX
Note that the position of TOOLBOX was indeterminate (undefined behavior) previously: the order of otherprojects, print_export, TOOLBOX seems to depend on the loading order of the extensions that add these portals (otherprojects: Wikibase, print_export: Collection, TOOLBOX: CiteThisPage, MachineVision and ShortUrl or UrlShortener).
@Krinkle the change we pushed is responsible. By adding toolbox and languages to Skin:buildSidebar we allowed these portals to be adjusted by the SidebarBeforeOutput hook
If we want to enforce a certain order we'd need to sort the associative array at the bottom of buildSidebar with rules. Alternatively, Vector itself should be updated to define the order. Timeless does something similar.
Note this change would mean it would not be possible for MediaWiki:Sidebar to reorder this under any circumstances. It would be a skin level decision.
Change 602891 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/core@master] Make languages always last in the sidebar
^ if we want to enforce languages as last and not movable by MediaWiki:Sidebar but editable by hooks (per T254546#6199464) the above patch should be suitable.
The problem with that change is that it uses one hook for actions at 2 different phases. In order:
Additionally, if an extension depends on another extension doing something in the hook then the execution order of the hooks is relevant, which results in undefined behavior.
For ex. ElectronPdf depends on Collections (code) being loaded before, thus executing its hook earlier and adding the 'coll-print_export' portlet (code) before ElectronPdf looks for it (code).
EDIT: Related: T256071: Collection extension should be made aware of the ElectronPdf extension: Settle on a common sidebar key for both extensions
Wikibase also adds the otherprojects section in the onSidebarBeforeOutput hook (code)
The previous hook in the sequence is onSkinBuildSidebar, but the result of that is cached, thus I assume it can't vary depending on the page and other factors, this is why phase 1 and 2 is done in only one hook.
To solve the issues of undefined behavior (load order dependence) and TOOLBOX added before extra extension sections, Phase 1 (add) should be a different hook from phase 2 (edit - onSidebarBeforeOutput).
Phase 1 could be called onSidebarPopulate or similar.
The change in the sidebar is observed at nowiki, and some in the community has disagreed enough that a gadget is created to restore the original behavior.
I don't have any particular preference on the ordering.
@jeblad No need for a gadget, just ending MediaWiki:Sidebar with this will restore the order:
... * query * store * wikibase-otherprojects * coll-print_export * TOOLBOX
The problem is that this change affected most wikis and now each have to add those two (otherprojects and print_export) above TOOLBOX to restore the order.
Change 603065 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] Restore Sidebar order: move TOOLBOX and LANGUAGES to the bottom
Change 602795 abandoned by Aron Manning:
[Abandoned] Call onSidebarBeforeOutput() hook before populating TOOLBOX and LANGUAGES
Reason:
In favor of https://gerrit.wikimedia.org/r/c/mediawiki/core/ /603065
Move both TOOLBOX and LANGUAGES to the bottom while respecting custom position if set in [[MediaWiki:Sidebar]]: patch 603065
This restores the original order before the refactoring of TOOLBOX hooks, regardless of wiki configuration.
The race condition between Collections and ElectronPdfService is not resolved by this, the task for that is:
T256071: Collection extension should be made aware of the ElectronPdf extension: Settle on a common sidebar key for both extensions
Change 603413 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/core@master] [POC] Add SidebarPopulate hook between SkinBuildSidebar and SidebarBeforeOutput
Change 603417 had a related patch set uploaded (by Aron Manning; owner: Aron Manning):
[mediawiki/extensions/Wikibase@master] [POC] Use SidebarPopulate hook to add 'In other projects' section before TOOLBOX
This is a more substantive proof-of-concept change, adding a missing hook as detailed in T254546#6199646. POC patch: 603413.
Using the first SidebarPopulate hook removes the need for reordering the sidebar for 'In other projects' added by Wikibase: patch 603417.
Also eliminates the race condition between Collections and ElectronPdfService: patch 603416.
Latter patches break compatibility of those extensions with <1.35 MediaWiki, that needs to be communicated to operators. Merging those changes at a later version might be a better option for a more relaxed update process.
Two more extensions use SidebarBeforeOutput hook to add a section before TOOLBOX: (codesearch): Insider, RelatedSites. These can be migrated to SidebarPopulate hook when appropriate.
Ainut adds a section after SEARCH. This is independent of the position of TOOLBOX, thus can be done in both hooks, needs no change.
Other extensions use SidebarBeforeOutput as intended: DynamicSidebar replaces dummy sections, ElectronPdfService modifies the 'coll-print_export' section, the rest modifies (extends) TOOLBOX.
@Nirmos the regression also moved "Tools" (aka. TOOLBOX) above "In other projects". What is the preferred order for these 3 portlets?
#p-wikibase-otherprojects ("In other projects")
#p-coll-print_export ("Print/Export")
#p-tb ("Tools")These suggestions don't sound bad, but it's hard to evaluate them without knowing which (if) one is the way it behaved previously, or why it can't. I'm confused by the arguments in this ticket and on the linked patches as it seems to imply a burden of proof on end-users to defend against an accidental change. "Maybe the toolbox is also fine here rather than there".
Maybe. But as I understand it, there has been no UX or design research that called for changing this in the first place. There also hasn't been a calculation of maintenance cost toward product mgmt to lay out the capalities and the benefit/downside trade-off it would present, how it would impact various wikis, how to prepare them for that change, and then making those changes. That isn't this ticket.
Was it intentional that the effective order as seen on any wiki is now different then before? No. Then why are not applying the same logic in the new system? Mixing refactoring with improved last-minute changes based on an incomplete understanding is generally not a path toward a stable foundation to build on.
I don't mean to come across as change resistant, but this isn't the time and place for new ideas. There was an incomplete understanding of the sidebar logic. Over the past two weeks this understanding has improved, so let's go back in now and apply that new knowledge. Once we have it working again and uncover no other unknowns, consider filing a ticket to changing it intentionally (if so desired).
I'm sorry for writing long analytic comments, unfortunately the interactions of extensions are quite complex.
TL;DR: The loading order of extensions can affect the position of the TOOLBOX and other portlets (sections): portlets are added to the sidebar either a) by MediaWiki:Sidebar or b) when an extension adds the first link to that portlet in onSidebarBeforeOutput().
The order in which extensions are loaded on wikimedia wikis resulted in the following sidebar order:
"In other projects", "Print/Export", "Tools" (TOOLBOX), "Languages". This was changed by the hook patch to the order below.
On most of the 133 wikis where TOOLBOX is listed in MediaWiki:Sidebar the order was:
"Tools" (TOOLBOX), "In other projects", "Print/Export", "Languages".
There are a few exceptions where TOOLBOX is not the last in MediaWiki:Sidebar.
After the patch that made the TOOLBOX editable in the onSidebarBeforeOutput() hook the order became (regardless of MediaWiki:Sidebar):
"Tools" (TOOLBOX), "Languages", "In other projects", "Print/Export".
... as it seems to imply a burden of proof on end-users to defend against an accidental change. "Maybe the toolbox is also fine here rather than there".
Was it intentional that the effective order as seen on any wiki is now different then before? No.
I don't mean to come across as change resistant, but this isn't the time and place for new ideas. There was an incomplete understanding of the sidebar logic. Over the past two weeks this understanding has improved, so let's go back in now and apply that new knowledge. Once we have it working again and uncover no other unknowns, consider filing a ticket to changing it intentionally (if so desired).
Very good reasoning. I couldn't have said better.
Change 602891 abandoned by Jdlrobson:
Make languages always last in the sidebar
Reason:
Needs more conversation on ticket. Will get to this wed/thur.
This should be fixed now. It will be fixed in Monobook in production in next train.
When I made these changes originally I was aware of the reordering during testing and felt it was a small price to pay for more readable consistent code and behaviour with a single hook making modifications. Given that it didn't go unnoticed, I was wrong and have learned from that. One thing I hadn't factored in was the length of the languages portal (since my testing of languages on my local wiki is suboptimal) and I agree that this was problematic as it pushed other sidebar options to the bottom.
Vector (legacy) already displays languages at the bottom following some refactors there in preparation for moving languages out of the sidebar (I4be06278ec256cc55aee36c2946339fb49d21b46). In Cologne Blue languages are in the top right. In Minerva it's in an overlay. In Timeless it's on this right. Monobook now shows this at the bottom after this patch.
If in the next release, there is of course the problem of 3rd party skins which run on wikis with multiple languages. Those skins will of course may require updates, however I am not expecting this to be a big hardship. If 3rd parties do report this as a big problem, I'm willing to reconsider and backport if necessary, but I feel my stubbornness is warranted here - right now the API is clear - all sidebar portals are created equally, treated equally and there is no special behavior for any of them. Skins if they want can display in the order they desire and skins can also allow editors to reorder in MediaWiki:Sidebar.
Our data shows most users ignore and don't use the sidebar entirely (it's mostly editors who I use). I don't think its wise to spend time doing UX or design research at this point for something that is fundamentally broken. There are better places to expend energy. This week we are building out instrumentation which will give us a lot more insight to this sidebar (T250282).
In the bigger picture, languages doesn't really belong in the sidebar. I also don't believe it is in our interest to ensure the toolbox always precedes the language portal. It complicates a lot of the logic in the skin that we've been trying to simplify and if a mediawiki setup is not using any extensions which add to the sidebar it will not see any difference.
As the person who identified the cause of this bug, communicated a temporary solution, made a thorough analysis of the cross-dependencies of affected extensions and submitted the first (and updated) fixes, I'd like to discuss the shortcomings of how this bug is handled.
The fix for all the regressions could have been fixed with minimal PHP code by Jun 17.
Instead, only half of the regression is fixed in just one skin (2 unaffected, 1 waiting for train) by today. Seemingly the other half of the regression (toolbox position change) - which is not as disruptive as the languages section - is denied to be recognized as a change that wasn't requested or planned.
The chosen resolution forces every skin to add the equivalent of the above patch or more complex code. That endeavour would cost hours of developer time from many people who had nothing to do with the refactorings, compared to accepting one already existing patch.
Besides, similar code would be repeated in many places. It was my understanding that WET is considered an anti-pattern here.
There are some unanswered questions:
The hook refactoring in itself is great progress, however, the PHP array initialization logic was misunderstood and some complications of the extension loading order were overlooked. The impact of the changes - if understood - was not communicated to communities and even when the bug report came in, volunteers were guessing the cause of the issue until I've pointed to the source.
All along the discussion of this bug, seemingly the burden to resolve the issue was pushed back to the users (T254546#6196971) suggesting a solution that requires action on hundreds of wikis, instead of a few lines of PHP code. The above suggested "resolution" forces many people to spend hours on a task that has no benefit and they didn't ask for.
The communities were not asked about what they want and seemingly the end result is one person's decision, ignoring all the effort (analysis, reasoning and solution) put into it. I don't think stubbornness is beneficial here.
I believe the handling of this issue negatively affects operators and other developers, unnecessarily.
We can do better than this if we collaborate.
Change 603413 abandoned by Ammarpad:
[mediawiki/core@master] [POC] Add SidebarPopulate hook between SkinBuildSidebar and SidebarBeforeOutput
Reason:
Change 603417 abandoned by Ladsgroup:
[mediawiki/extensions/Wikibase@master] [POC] Use SidebarPopulate hook to add 'In other projects' section before TOOLBOX
Reason:
The author has been banned indef from our tech spaces and the patch has not been touched for really long time.
Change 603065 abandoned by Ladsgroup:
[mediawiki/core@master] Restore Sidebar order: move TOOLBOX and LANGUAGES to the bottom
Reason:
The author has been banned indef from our tech spaces and the patch has not been touched for really long time.