Page MenuHomePhabricator

Add-portlet-link gadgets and scripts in "more" menu are missing in Timeless
Closed, ResolvedPublic

Description

Hello. And that's because of different ids of regular "more" items.

Event Timeline

Izno renamed this task from All add-portlet-link gadgets and scripts in "more" menu are broken. to Add-portlet-link gadgets and scripts in "more" menu are missing in Timeless.Nov 26 2017, 6:12 PM
Izno subscribed.

This is the general issue w.r.t. T181206: Can't find Twinkle in Timeless skin.

Isarra triaged this task as High priority.Nov 30 2017, 6:37 PM

A work-around I've used in one of my scripts is checking the skin type and adjusting the portlet location accordingly, i.e. changing

mw.util.addPortletLink( 'p-cactions', '#', 'Foo', 'ca-foo', 'Foo bar baz qux' );

to

mw.util.addPortletLink(
    ( mw.config.get('skin') === 'timeless' ) ? 'p-pageactions' : 'p-cactions',
    '#',
    'Foo',
    'ca-foo',
    'Foo bar baz qux'
);

which is a little bit annoying to have to do, but it works.

I wonder if it makes sense/is possible to add a JavaScript method which allows a skin agnostic location for portlet manipulation and then let the JavaScript loaded with the skin figure out where to place the links. Otherwise this is going to be an issue every time a new skin comes along with not-quite-the-same-IDs/classes.

(Maybe the more-used scripts should be implemented as extensions. Doesn't fix the bigger issue of course.)

Wouldn't it be a lot simpler, in the short term at least, for Timeless to reuse the same ids that all the other skins available on Wikimedia wikis (apart from Minerva/mobile) use, rather than inventing its own? I mean, its already doing so for a whole lot of other portlets (e.g. p-personal, p-tb, p-navigation, p-interaction), so why not change either p-pageactions or p-pagemisc to p-cactions?

(And maybe also create some sort of "Guide to making a new skin" to detail lessons learned from creating Timeless, which could include that for most existing userscripts/gadgets to work on your new skin, you should include portlets with standard ids.)

Great idea. If anyone wants to make a patch for that, please just yell at me and I'll merge it.

Change 399783 had a related patch set uploaded (by Evad37; owner: Evad37):
[mediawiki/skins/Timeless@master] Change pagemisc portlet to cactions

https://gerrit.wikimedia.org/r/399783

Change 399783 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Change pagemisc portlet to cactions

https://gerrit.wikimedia.org/r/399783

In https://gerrit.wikimedia.org/r/#/c/399783/, @Isarra wrote:

Actually, given the use case (adding tabs and often dropdowns), maybe it would be better to change pagetools instead of pagemisc? They are usually tabs, right?

Its a little bit complicated:

  • In Vector, there are separate portlets for namespace/ns_talk (namespaces), read/edit/history/watch (views), and actions like move (cactions). The first two are tabs at the top of the page, the third is a dropdown menu from a tab labeled "More".
  • In Monobook and Modern, these are all in one single portlet cactions displayed as tabs at the top of the page.
  • In CologneBlue, namespace/ns_talk and history are combined as pageoptions, and edit and move are combined as cactions. Both are displayed as a list of links in the sidebar.
  • Timeless is much like Vector, but: watch joins namespace/ns_talk in namespaces; read/edit/history is called pagetools; move is in pageactions.

So for the most compatibility pagetools should become views, and pageactions should become cactions.

But if we did that, there would have to be another bug for cactions not always being present: the conditional on line 404 means that the portlet isn't present on some pages (e.g. Special:Watchlist), and therefore scripts won't work if they try to add portlet links on such pages (they can do so on Vector because it includes the portlet elements on the page but with an empty <ul></ul> inside).

Looking at the source code for mw.util.addPortletLinks (mediawiki.util.js line 336), it seems that empty portlets should be present but hidden with css class emptyPortlet, which then gets removed when the function is called.

Change 399977 had a related patch set uploaded (by Evad37; owner: Evad37):
[mediawiki/skins/Timeless@master] Changes to portlets for greater compatibility with other skins

https://gerrit.wikimedia.org/r/399977

@Isarra have you seen this latest patch?

Yeah, I've been meaning to come back to this when my brain is actually working, but it hasn't happened for whatever reason. I've asked @ashley to take a look in the meantime, though - if everything's sane he can totally merge it anyway. >.>

Change 399977 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Changes to portlets for greater compatibility with other skins

https://gerrit.wikimedia.org/r/399977

Sorry, I was on the wrong computer for awhile there and it kind of got lost in backlog.

That patch does seem to mess up ULS's handling (forced visibility) of empty languages blocks. I'm not sure what to make of it, though, so whatever.

Change 404619 had a related patch set uploaded (by Evad37; owner: Evad37):
[mediawiki/skins/Timeless@master] Make languages portlet visible when empty

https://gerrit.wikimedia.org/r/404619

Change 404619 merged by jenkins-bot:
[mediawiki/skins/Timeless@master] Make languages portlet visible when empty

https://gerrit.wikimedia.org/r/404619