Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Page MenuHomePhabricator

Allow specifying when a gadget should load (action, namespace, content model)
Closed, ResolvedPublic

Description

Gadgets, when enabled by a user, load on all pages in all namespaces. This doesn't cause any immediate functional problems, but is inefficient. In some cases, very unacceptably inefficient, to the point that a gadget may be removed from a wiki or disabled due to performance problems. This is usually worked around in one of two ways:

  1. The gadget is split into two parts. A part that contains the actual gadget itself (hidden, disabled by default). And a (tiny) second part that is loaded always and checks whether the current page is the "right" one, and then loads the other.
    • Downside: Two definitions instead of one.
    • Downside: The page will first load without the gadget, and then later the gadget gets loaded (typically between 1-10s seconds later, depending on the connection and bandwidth).
  2. The gadget is made hidden and is instead loaded by url parameter (e.g. /wiki/Example?withModule=ext.gadget.foo).
    • Downside: Cannot be enabled on a per-user base (url parameter applies to all users). It also means the gadget only loads when the user follows a specific link, e.g. not when visiting the page through other means.
    • Downside: The page will first load without the gadget, and then later the gadget

We should instead add an option directly in the Gadgets extension to specify when a gadget should load (by default still everywhere).
Eg. based on the namespace, or page title, or page action, or canonical special page name.


Original request:

Author: gryllida

Description:
We should add an option to specify what pages a gadget is active on. Many probably just do this manually, but it looks reasonable as some gadgets would only be useful for contributors' talk pages for example. This could be a namespace name, or sometimes even a glob mask (Wikipedia talk:Articles for Creation/*).

See Also:

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

Change 747112 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/Gadgets@master] Allow specifying action pages in definitions

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

Change 747112 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Allow specifying page actions in definitions

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

@Johan, what do you say, do we need to publish this?

@IKhitron: In case you're thinking about Tech News (not sure), please see https://meta.wikimedia.org/wiki/Tech/News/For_contributors - thanks!

Don't understand what are you trying to say.

Is there a list of all accepted action values somewhere? E.g. is there a way to load a gadget only when viewing diffs?

Is there a list of all accepted action values somewhere? E.g. is there a way to load a gadget only when viewing diffs?

Diffs generally qualify as action=view with, depending on URL parameters and preferences, a diff and below it either the current content or a previous revision content or no content ("diffonly").

Keep in mind that these conditions exist solely for the purpose of optimising payload performance. The actual activation or intialisation of functionaly must be based on checks or hooks inside the gadget code (e.g. mw.hook('wikipage.diff') and other hooks). As such, we do not need a perfect match for every scenario.

Also keep in mind that once loaded at least once, the code remains cached in the browser until the gadget is edited. So the main thing we're optimising isn't per-se what loads on a page, but what is loaded for a person in a given browsing session on a given day. E.g. if someone is likely never to need the code that day, don't load it.

With this in mind, the main category we are optimising for is simple article namespace page views vs everything else. For diffs we could go in a number of directions:

  1. We could load gadgets for action=history on diffs (in addition to action=view gadgets). This has the benefit of avoiding overlap between action values, but the downside of exposing the true (but otherwise mostlyh hidden) detail that the conditions are only for optimising, not for hard enforcement. That is, you cannot assume that a gadget will only ever load in the specified action. It can also be loaded via mw.loader.using() or dependencies= anywhere else. And if we go with loading action=history on diffs, then it will be true in that case as well.
  1. We could introduce a fake action value of action=diff. This would mean on diffs we load action=view and action=diff. This has the downside of making the action parameter no longer something you can know and understand based on general MediaWiki knowledge and instead now requires discovering and learning the special meaning in the Gadgets extension. Also may cause conflicts in the future, and will forever cause a small level of confusion. But it has the benefit of allowing to load something only for diffs and not on history page. I don't know if that is significant however, as it can be a rather effective means of preloading. People generally spend a few seconds on the history page deciding what to do. Loading it in the background there into the cache before clicking on a diff seems useful.

Change 922062 had a related patch set uploaded (by SD0001; author: SD0001):

[mediawiki/extensions/Gadgets@master] Add support for content models in definitions

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

Is this still stalled on lack of an active maintainer for Extension:Gadgets?

Jdlrobson raised the priority of this task from Medium to High.Aug 10 2023, 6:36 PM
Jdlrobson subscribed.

Bumping this to high given T340705. I'll look to prioritize finding this a maintainer.

Change 624517 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add support for namespaces in definitions

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

Thanks for implementing this! I'm sure this is on your todo list but don't forget to document this on https://www.mediawiki.org/w/index.php?title=Extension:Gadgets&safemode=1#Options !

Hi, please could someone suggest what wording and links to use as the Tech News summary? (1–3 sentences, 1–2 links). Thanks!
(P.s. Drafts always help, otherwise I have to read and understand entire task-threads!)
My best-guess, from looking at the gerrit-patch description, would be:

Changes this week

  • Gadget definitions will have a new "namespaces" option. The option takes a list of namespace IDs. Gadgets that use this option will only load when the viewed page is in one of the required namespaces.

@Quiddity That is pretty much spot on! However, would suggest changing will only load when the viewed page is in one of the required namespaces -> will only load on pages in the given namespaces for simplicity.

Just mentioning this here so it's not lost in the shuffle:

In T204201#9127376 I describe a use case that needs OR semantics for conditions; specifically to load a charinsert-ish Gadget on all pages with contentmodel wikitext and on Special:Upload (which has a textfield with wikitext for the file description page). Iow, it needs to load on (a given) special page(s) OR on wikitext pages.

It's a slightly different scope than this task—and I'm guessing rather harder to implement—but I think it's important to keep in mind for those with their fingers (and minds) deep in this area.

@SD0001 Thanks! Added to https://meta.wikimedia.org/wiki/Tech/News/2023/39 - Note that is linking to the aforementioned https://www.mediawiki.org/wiki/Special:MyLanguage/Extension:Gadgets#Options which I believe needs to be updated with the exact new parameter(s).

In T204201#9127376 I describe a use case that needs OR semantics for conditions; specifically to load a charinsert-ish Gadget on all pages with contentmodel wikitext and on Special:Upload […] I think it's important to keep in mind for those with their fingers (and minds) deep in this area.

The filtering options in Gadgets exist for one reason and one reason only: performance, not for application security or business logic correctness. In other words: If we were to temporarily remove a filter option or make all filters temorarily return "true", if that causes anything other than unused JS code, there is a bug in the gadget.

The options dictate where modules load automatically. They can still be loaded indirectly as dependency, ad-hoc, or through cost-saving measures if a filter option becomes too expensive to calculate and we might round toward loading anyway.

You can take your pick from the options available to make the biggest savings, and use JS logic to enforce specific requirements as needed for correctness.

I think one thing we can do here, given that page actions are special pages with actions (diagram: an index.php request is routed to either an Action handler or a SpecialPage handler), is to perhaps treat "specials" as a symbolic value in the actions filter.

That way you could narrow your gadget to actions=edit,specials. With the primary goal being to not load it during main-namespace page views. Anything else we save toward perfection is small in comparison.

For the performance sake could you support namespace!=0 syntax?

I've made a quick proof of concept that is passing unit tests, but I don't have a full installation to really test it with. I can make a proper patch if that looks like something you could add to the extension.

If supporting negation perhaps it should be thought about more broadly and apply to all options (see also T342532)

This comment was removed by Izno.

I think one thing we can do here, given that page actions are special pages with actions (diagram: an index.php request is routed to either an Action handler or a SpecialPage handler), is to perhaps treat "specials" as a symbolic value in the actions filter.

That way you could narrow your gadget to actions=edit,specials. With the primary goal being to not load it during main-namespace page views. Anything else we save toward perfection is small in comparison.

It wouldn't help in this gadget's case, (since separate parameters are logical AND), so IDK if we necessarily want to jump that direction, but |namespace=-1 also would fit how special pages are dealt with (if that isn't already supported). I think treating them as actions makes about as much sense as any, but since we have some fidelity with actions actually specified as such, maybe take this a bit further and support something like |actions=special-upload, aiming to support the lowercase special pagename. Or the canonical name is fine e.g. |actions=special-RecentChanges.

[…] you could narrow your gadget to actions=edit,specials. […]

[…] but |namespace=-1 also would fit how special pages are dealt with (if that isn't already supported).[…]

namespace=-1 should already work indeed. The reason I suggest also supporting specials as an action is because a number of gadgets enhance the functionality of wikitext textareas, which appear on action=edit in many different namespaces, and on special pages like Special:Upload. action=edit would mean we (correctly) avoid loading the gadget on page views, but also (currently) makes it impossible to load on Special:Upload. namespaces=0,-1 would correctly load both on edit page and upload page, but would also load during page views. From a routing perspective, special pages are the equivalent of action="" or action=null, but to avoid hardcoding that or locking into no other possibilities in the future, I was thinking action=specials could represent that for completion within the Gadgets extension.

Anyway, if that doesn't work for your case, no problem. I'd rather not increase complexity until there's a use case that needs it, and benefits performance in a way that can't be achieved by other means already.

Change 922062 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add support for content models in definitions

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

Change 960156 had a related patch set uploaded (by Krinkle; author: Krinkle):

[mediawiki/extensions/Gadgets@master] Improve localisation on Special:Gadgets, minor code cleanups

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

[…] you could narrow your gadget to actions=edit,specials. […]

[…] but |namespace=-1 also would fit how special pages are dealt with (if that isn't already supported).[…]

namespace=-1 should already work indeed. The reason I suggest also supporting specials as an action is because a number of gadgets enhance the functionality of wikitext textareas, which appear on action=edit in many different namespaces, and on special pages like Special:Upload. action=edit would mean we (correctly) avoid loading the gadget on page views, but also (currently) makes it impossible to load on Special:Upload. namespaces=0,-1 would correctly load both on edit page and upload page, but would also load during page views. From a routing perspective, special pages are the equivalent of action="" or action=null, but to avoid hardcoding that or locking into no other possibilities in the future, I was thinking action=specials could represent that for completion within the Gadgets extension.

Anyway, if that doesn't work for your case, no problem. I'd rather not increase complexity until there's a use case that needs it, and benefits performance in a way that can't be achieved by other means already.

Ah, don't mistake me, I support supporting special pages as actions -- it's that hitting namespace=-1 wouldn't work for the one specific gadget I think raised this discussion point. I do also support/prefer selecting specific special pages as actions, which is the part of the paragraph you snipped. :)

Does this mean that contentmodel dispersion will also work from the next deployment?

If supporting negation perhaps it should be thought about more broadly and apply to all options (see also T342532)

Hm... I guess the parser part can be more universal. But I think having gadgets not in main namespace would help with budgets etc more. I assume most people, most of the time, probably visit only the main namespace.

But I prepared a more universal code:

Still rough, but you could easily add support for other options (for those that it makes sense). E.g. skins:

	public function isSkinSupported( Skin $skin ) {
		if ( !empty( $this->requiredSkins ) ) {
			$isNegative = isset( $this->negatedOptions['skins'] );
			$isIn = in_array( $skin->getSkinName(), $this->requiredSkins, true );
			return $isNegative ? !$isIn : $isIn;
		}
		return true;
	}

I've added temporary links at the Extension:Gadgets page, pointing here, until someone can properly document the new feature(s) in the table there. Please fix/improve if you can. Pinging main patch authors (IIUC) @BrandonXLF and @SD0001 in case you can help. Thanks!

Krinkle renamed this task from Allow specifying when a gadget should load (conditional category, action, or namespace) to Allow specifying when a gadget should load (action, namespace, content model).Oct 25 2023, 8:48 PM

Change 1005092 had a related patch set uploaded (by Sophivorus; author: Sophivorus):

[mediawiki/extensions/Gadgets@master] Add support for categories in definitions

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

Change 1005092 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Add support for categories in definitions

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

Krinkle claimed this task.

Change #960156 merged by jenkins-bot:

[mediawiki/extensions/Gadgets@master] Improve localisation on Special:Gadgets, minor code cleanups

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

[…] but |namespace=-1 also would fit how special pages are dealt with (if that isn't already supported).[…]

namespace=-1 should already work indeed. The reason I suggest also supporting specials as an action is because a number of gadgets enhance the functionality of wikitext textareas, which appear on action=edit in many different namespaces, and on special pages like Special:Upload. action=edit would mean we (correctly) avoid loading the gadget on page views, but also (currently) makes it impossible to load on Special:Upload. namespaces=0,-1 would correctly load both on edit page and upload page, but would also load during page views. From a routing perspective, special pages are the equivalent of action="" or action=null, but to avoid hardcoding that or locking into no other possibilities in the future, I was thinking action=specials could represent that for completion within the Gadgets extension.

There are many gadgets that are intended to be used on only one (or a few) specific special page (Special:Watchlist, Special:Contributions, etc.). There are even gadgets intended to be used on one specific regular (i.e. non special) page.

Therefore, I would suggest adding an option |pages=.... I think this option would be very handy in many cases.

One minor question, should it follow the redirects? (I'd lend to answer no, to keep the code straightforward)

Change #1077955 had a related patch set uploaded (by Sophivorus; author: Sophivorus):

[mediawiki/extensions/Gadgets@master] Add support for templates in definitions

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

Hi! I just submitted 1077955 to add support for templates in the gadget definitions. After the merge of 1005092, me and a few others developed several JavaScript-enhanced templates (see template gadgets and global gadgets). However, although loading gadgets via categories is technically enough, it leads to the creation of many obscure and otherwise useless categories (see every subcategory here and here). This is unnecessary and cumbersome, it would be better if we could load gadgets based on the presence of specific templates, which is generally the intended functionality anyway.

I still think T241524: Parser function for loading gadgets is the way to go, not this. If a template with a template gadget has a sandbox, there are two options:

  • The sandbox is also added to |templates=. This is impossible if you’re not an interface admin.
  • Instead of the main template, a subtemplate has been added to |templates=, which does nothing except for triggering the gadget load (so that the same version can be safely used in the live template and the sandbox). This is no better than those useless categories, except that it appears two lines higher up at the bottom of the edit window than a hidden category.

Yup, <templatescripts> (akin to <templatestyles>) seems much cleaner.

Sure, other approaches may be (marginally) better, but realistically speaking, will take years to happen. This approach can be implemented now and wouldn't block or hinder the development of better alternatives. If anything, it would give it some extra push by helping the development of actual tools to be loaded by said alternatives.

@Tacsipacsi Regarding the sandboxes of templates with template gadgets, when I work in such sandboxes, I generally don't want to load the same gadget as the main template, but a modified version of the gadget, which I usually store in a subpage of my userpage and load from my common.js

I still think T241524: Parser function for loading gadgets is the way to go, not this. If a template with a template gadget has a sandbox, there are two options:

  • The sandbox is also added to |templates=. This is impossible if you’re not an interface admin.
  • Instead of the main template, a subtemplate has been added to |templates=, which does nothing except for triggering the gadget load (so that the same version can be safely used in the live template and the sandbox). This is no better than those useless categories, except that it appears two lines higher up at the bottom of the edit window than a hidden category.

I still think that's the wrong approach and will be a headache down the road to manage. Supporting templates is reasonable (and really how "category" should have been done in the first place) if we're going down this road (and incidentally using a similar approach on the templatelinks table avoids some of the drawbacks needing to be fixed about the category approach discussed in that task).

The only benefit with supporting templates is that using them in interface messages won't weirdly cause a category to show up at the bottom in unexpected contexts like special pages. It also doesn't need a row in categorylinks, sure, but I suspect communities would toss in a category anyway for tracking.

I still think T241524: Parser function for loading gadgets is the way to go, not this ...

I still think that's the wrong approach and will be a headache down the road to manage.

How so? It only becomes easier to manage down the road when backlinks of JSON definition pages can be used for tracking.