Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix path to ext.SimpleMathJax.js in ResourceModules (#1) #40

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

pastakhov
Copy link
Contributor

Mediawiki tries to load extensions/SimpleMathJax/resources/resources/ext.SimpleMathJax.js file when ResourceLoader in debug mode, the patch fixes the path

@TripleCamera
Copy link

TripleCamera commented Apr 1, 2024

@jmnote Hi. Could you please review this Pull Request? This issue is occurring on a wiki I know, which upgraded to MediaWiki 1.41 a few days ago. (I have no idea why it doesn't affect other wikis.)

@jmnote
Copy link
Owner

jmnote commented Apr 5, 2024

Thanks for the PR. But I'm not sure if it's right to change it that way. I don't see any problems with the existing code. Would you like to check out the following examples and documentation?

	"ResourceFileModulePaths": {
		"localBasePath": "resources",
		"remoteExtPath": "FooBar/resources"
	}

https://www.mediawiki.org/wiki/Manual:Extension.json/Schema

	"ResourceFileModulePaths": {
		"localBasePath": "modules",
		"remoteExtPath": "CodeEditor/modules"
	},

https://github.com/wikimedia/mediawiki-extensions-CodeEditor/blob/wmf/1.40.0-wmf.4/extension.json#L139-L142

Is it possible that it is affected by other configs? I think you might need to check the extension.json file. Also, if the problem occurs only in version 1.41, we should check the release notes.
https://mediawiki.org/wiki/Release_notes/1.41

@TripleCamera
Copy link

TripleCamera commented Apr 5, 2024

Hi. A few days ago, the webmaster told me that the issue of his wiki failing to load JS modules was caused by other extensions. Sorry that I didn't update my comment in advance.

According to @AlPha5130, for SimpleMathJax, the value of localBasePath is correct, while the value of remoteExtPath is incorrect. The former one is responsible for loading the modules when browsing the wiki, and the latter one is responsible for the URL endpoint, which is provided to anyone who may concern.

"ResourceModules": {
"ext.SimpleMathJax": {
"scripts": ["resources/ext.SimpleMathJax.js"],
"targets": ["desktop", "mobile"]
}
},
"ResourceFileModulePaths": {
"localBasePath": "",
"remoteExtPath": "SimpleMathJax/resources"
},

Currently, the URL endpoint is /w/extensions/SimpleMathJax/resources/resources/ext.SimpleMathJax.js (remoteExtPath + scripts), but it is expected to be /w/extensions/SimpleMathJax/resources/ext.SimpleMathJax.js. Nobody has used this endpoint so far, and MediaWiki only tries to load this file in debug mode (append ?debug=1 to URL). So for now, this is not causing any problem, but fixing it would be better. 😊

PS. MediaWiki extension Purge just fixed this a few days ago. You can see it here.

@jmnote
Copy link
Owner

jmnote commented Apr 6, 2024

This issue involves deciding whether or not to enter resources in three places as shown below.

"ResourceModules": {
  "ext.SimpleMathJax": {
    "scripts": ["(resources/???)ext.SimpleMathJax.js"],
    ...
"ResourceFileModulePaths": {
  "localBasePath": "(resources???)",
  "remoteExtPath": "SimpleMathJax(/resources???)"

A total of 8 $(=2^3)$ test cases were tested by expressing the number of all cases bitwise as follows. (My testing environment: MediaWiki 1.39)

  • case 000: RuntimeException: script file not found or not a file
  • case 001: RuntimeException: script file not found or not a file
  • case 010: OK
  • case 011: OK (AlPha5130's PR for Purge, Manual:Extension.json/Schema)
  • case 100: OK (@pastakhov 's PR)
  • case 101: OK (AS-IS, current main branch)
  • case 110: RuntimeException: script file not found or not a file
  • case 111: RuntimeException: script file not found or not a file

What we see in cases 000, 001, 110, and 111 is that we must enter resources only in either scripts or localBasePath.

Also, it has been reported that case 101, the current state (main branch), has a problem.
We have to choose from the remaining cases 010, 011, and 100.
The current PR corresponds to case 100, but it would be better to refer to Manual or Purge and modify it as follows according to case 011. What do you think?

"ResourceModules": {
  "ext.SimpleMathJax": {
    "scripts": ["ext.SimpleMathJax.js"],
    ...
"ResourceFileModulePaths": {
  "localBasePath": "resources",
  "remoteExtPath": "SimpleMathJax/resources"

@jmnote jmnote self-assigned this Apr 6, 2024
@AlPha5130
Copy link

AlPha5130 commented Apr 7, 2024

The current PR corresponds to case 100, but it would be better to refer to Manual or Purge and modify it as follows according to case 011. What do you think?

It depends on how you would organize the JS file hierarchy. In my case (Purge) I set the "root" path (i.e. localBasePath) to resources as all my JS files will be put there. If you have other JS files which do not fit in resources you can set localBasePath to "".

Copy link
Owner

@jmnote jmnote left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks~

@jmnote jmnote merged commit 0b5e165 into jmnote:main Jul 2, 2024
DanielWTQ added a commit to WikiTeq/Taqasta that referenced this pull request Nov 29, 2024
Upstream added our patch (jmnote/SimpleMathJax#40) and there are also a few
other trivial improvements since then

WIK-1761
DanielWTQ added a commit to WikiTeq/Taqasta that referenced this pull request Dec 1, 2024
Upstream added our patch (jmnote/SimpleMathJax#40) and there are also a few
other trivial improvements since then

WIK-1761
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants