@Samwilson and I looked at this. We'd like to confirm that the solution would look like this:

• Move resources/src/mediawiki.diff.styles/diff.js to resources/src/mediawiki.action/diff.js
• In core's mediawiki.action.history, replace 'scripts' with

'skinScripts' => [
    'default' => [ 'resources/src/mediawiki.action/mediawiki.action.history.js', 'resources/src/mediawiki.action/diff.js' ]
]

• Remove the mediawiki.diff module and update PHP code accordingly

Then, if it was a CSS file, we could edit Minerva's skin.json, and under 'ResourceModuleSkinStyles', add

"mediawiki.action.history": [ "resources/src/mediawiki.action/mediawiki.action.history.js" ],

However, there doesn't seem to be an equivalent of ResourceModuleSkinStyles for scripts.

As such, I guess the idea would be to do the following in core instead:

'skinScripts' => [
    'default' => [ 'resources/src/mediawiki.action/mediawiki.action.history.js', 'resources/src/mediawiki.action/diff.js' ],
    'minerva' => [ 'resources/src/mediawiki.action/mediawiki.action.history.js' ],
]

the main concern here is that core would be explicitly referring to a skin.

What about there being some way in the javascript to be able to detect if a page view is desktop or mobile (maybe there already is such a way) - then the diff.js could check that to determine if it should run

In T291433#7403611, @DannyS712 wrote:

What about there being some way in the javascript to be able to detect if a page view is desktop or mobile (maybe there already is such a way) - then the diff.js could check that to determine if it should run.

The distinction here is not so much the device being used, but specifically whether MobileFrontend/Minerva have modified the page which is not exactly the same. There are ways to check skin and MFMode in mw.config, but references to such variables do not belong in core (separation of concerns, and fair/equal oppertunity for other skins).

If the logic has to be in core, than using the existing skinScripts as shown by Daimona would be simpler and more performant than conditionally executing a package file.

The preferred approach is indeed for Minerva to signal to core to skip this script, rather than for core to signal to Minerva.

In T291433#7382981, @Daimona wrote:

@Samwilson and I looked at this. We'd like to confirm that the solution would look like this:

• Move resources/src/mediawiki.diff.styles/diff.js to resources/src/mediawiki.action/diff.js
• In core's mediawiki.action.history, replace 'scripts' with

'skinScripts' => [
    'default' => [ 'resources/src/mediawiki.action/mediawiki.action.history.js', 'resources/src/mediawiki.action/diff.js' ]
]

• Remove the mediawiki.diff module and update PHP code accordingly

Sounds good to me.

[…] if it was a CSS file, we could edit Minerva's skin.json, and under 'ResourceModuleSkinStyles', add

"mediawiki.action.history": [ "resources/src/mediawiki.action/mediawiki.action.history.js" ],

It can be simpler by setting it to []. The script that we load always can remain in scripts and the ones that are optional for the skin, in skinScripts. Afaik we can't reference a core script from a different repository in a stable and supported manner. As-is, it would be looking for a file by this path within the local repository instead of core. And working around that, it'd still be fragile and violate separation of concerns in the other direction.

However, there doesn't seem to be an equivalent of ResourceModuleSkinStyles for scripts.

There is indeed not a way to set SkinScripts from skin.json, but I thought there was a way to set the configuration variable equivalant of it. Similar to how we have $wgResourceModuleSkinStyles (which has been around for a long time) and skin.json ResourceModuleSkinStyles (which came later).

But.. upon looking at it again now, it seems we never added the configuration variable version of it either. That's... unfortunate, and another reason to prioritise T250853.

In T291433#7403611, @DannyS712 wrote:

What about there being some way in the javascript to be able to detect if a page view is desktop or mobile (maybe there already is such a way) - then the diff.js could check that to determine if it should run

In which case, you'd be sending a resource to a client that will never use it. Which, considering the percentage of mobile request, is not exactly what I'd call a performance improvement.

In T291433#7403689, @Krinkle wrote:

The distinction here is not so much the device being used, but specifically whether MobileFrontend/Minerva have modified the page which is not exactly the same.

I should note that there are two reason why we're not serving this script to mobile users:

1. (primary) MF has its own diff style, which for WMF sites (at least) is unidiff, and there's no concept of "locked side" there
2. (secondary) The side locking logic is listening to mousedown/selectionchange events happening in specific ways/places, and I'm not sure how that'd work on mobile (I didn't test that)

[…] if it was a CSS file, we could edit Minerva's skin.json, and under 'ResourceModuleSkinStyles', add

"mediawiki.action.history": [ "resources/src/mediawiki.action/mediawiki.action.history.js" ],

It can be simpler by setting it to []. The script that we load always can remain in scripts and the ones that are optional for the skin, in skinScripts.

Oh, I should note that Sam and I found another bug while testing this. Using the empty array in skin.json would result in a JS error, since I think it still tries to load a script (maybe using the empty string as file name? I'd have to check again). The same bug doesn't happen if the empty array is used for skinScripts in core's Resources.php.

But.. upon looking at it again now, it seems we never added the configuration variable version of it either. That's... unfortunate, and another reason to prioritise T250853.

Just to confirm, does this mean that this task is blocked on T250853?

In T291433#7404150, @Daimona wrote:

Oh, I should note that Sam and I found another bug while testing this. Using the empty array in skin.json would result in a JS error, since I think it still tries to load a script (maybe using the empty string as file name? I'd have to check again). The same bug doesn't happen if the empty array is used for skinScripts in core's Resources.php.

I tried again and couldn't reproduce this bug. Perhaps it was just caused by our experiments.