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

Add as a gadget? #15

Open
samwilson opened this issue Nov 3, 2017 · 12 comments
Open

Add as a gadget? #15

samwilson opened this issue Nov 3, 2017 · 12 comments

Comments

@samwilson
Copy link
Contributor

Almost every Wikimedia wiki has a 'purge' gadget of some sort (for example).

Would it be a good feature to add to this extension, to make it available as a gadget? It'd still need to be added to the gadgets-definition page, of course, with just a dependency:

* purge[dependencies=ext.purge]|

We'd have to add a feature flag to turn off the purge tab by default, so that it'd only be enabled by gadget.

This would mean that all the different versions of the purge gadget would be the same, and work as well as they can. (Some of these gadgets for example do not do the POST trick, so people just get taken to ?action=purge and have to click the button. So tiring!)

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 4, 2017

So, there is a simple way to stop the GO button.

$wgGroupPermissions['*']['purge'] = true;

Most are set to user only and that is the default setting. Only someone in the user group can purge without the GO button. See https://www.mediawiki.org/wiki/Manual:User_rights#List_of_permissions at the bottom under technical. Important if you set it to all, you need the GO to stop bots from constantly flushing the file cache and using server resources.

Now that I am looking at this, what is resources/ext.purge.js actually doing? This was never changed, the href to create the link in the Actions.

			$content_actions['actions']['purge'] = array(
				'class' => $action === 'purge' ? 'selected' : false,
				'text' => wfMessage( 'purge' )->text(),
				'href' => $title->getLocalUrl( 'action=purge' )
			);

I guess what I would expect is href pointing at an onclick function, not building he hard link to an action=purge.

@samwilson
Copy link
Contributor Author

I'd forgotten about that permission; good point.

My point was more about there being no way to currently turn off the purge tab, short of removing the user right or uninstalling the extension. If it were installable as a gadget, it would be off for everyone by default (e.g. $wgPurgeIsGadget = true or something) but be able to be turned on user-by-user as a gadget.

This would also mean that the actions link would have to be optionally constructed in javascript.

I guess what I would expect is href pointing at an onclick function, not building he hard link to an action=purge.

The click action is intercepted in javascript, and e.preventDefault(); is run. This is a good way to do it, because it still works when javascript is disabled.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 5, 2017

The click action is intercepted in javascript, and e.preventDefault(); is run. This is a good way to do it, because it still works when javascript is disabled.

Ok, so the click is intercepted. If I replace the href with anything, like href => '#' js should execute the onclick function to reload the page since it should be attached to the element, but it isn't purging and reloading the page. I have examined all the js loaded and I can't find the script. With resource loading debug on, not in the list.

image

@samwilson
Copy link
Contributor Author

I guess you don't even need the href attribute at all, if you just want to intercept the click. But I think it'd be better to leave it in and keep the current behaviour for non-JS browsers.

I'm not sure what you mean by the module not loading, it works for me with the current version. Does your user perhaps not have the purge permission? Or do you mean after modifying it to also be able to add the purge link from JS and not only from the SkinTemplateNavigation hook?

(And I take it you do think it's a good idea to have Gadgets support?)

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 5, 2017

I guess you don't even need the href attribute at all, if you just want to intercept the click. But I think it'd be better to leave it in and keep the current behaviour for non-JS browsers.

I'm in favor of leaving it in. I just was trying to test the onclick function. What I'm not seeing is the JS resource module loading the JS. The screenshot is with resource loader debug on and the module should show as ext.purge.js?{a key} loading and it is not. What skin are you testing with? I am not using Vector. Could you test with $wgResourceLoaderDebug = true; and another skin, turning off Vector completely.

Does your user perhaps not have the purge permission?

Yes or the purge link in the action menu would not appear. For some reason

$skin->getOutput()->addModules( 'ext.purge' );

Is not loading the JS.

(And I take it you do think it's a good idea to have Gadgets support?)

I do, but we need to make sure the JS is loading from the module.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 5, 2017

Just edited ....

What I'm not seeing is the JS resource module not loading the JS

to... What I'm not seeing is the JS resource module loading the JS.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 5, 2017

Ok, I did a quick rewrite and pushed a branch, correctHook. I think the issue is the JS can't load with the addModule because it is too late to modify the output of the page by the time it is called. Needed to use another hook to make the last minute modification to output.

Testing this further, I added another hook (onBeforePageDisplay), reverted the other back to SkinTemplateNavigation. I am NOW SEEING the JS load. Yay!

image

Test results on branch:

  • I added a test for $wgUser->isAllowed() (yep, quick and dirty) to block loading of the module if they aren't allowed. Kept in the NS_Special test. The result is as expected, loads for users with permission, and doesn't for users without or on Special pages.
  • I also (and I left it now for testing) modified the output of the <href> to just add a # which basically goes nowhere. The click is intercepted and the purge action is executed by the JS in the loaded module. This was something I was not seeing before.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 5, 2017

Ah probably should have added testing on MW 1.27. All wikis I Admin are on LTS 1.27 which seems to be a good branch to test old vs new on.

If we bump this, I would like to see the extension function on MW 1.25>.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 7, 2017

@samwilson thoughts?

@samwilson
Copy link
Contributor Author

Sorry, I'm not quite sure what you're getting at. Are you saying that these changes are what's required to get this working as a Gadget?

Or do you mean that you're not able to get the current master to load the JS module? It works correctly for me. As far as backwards-compatibility goes, I would suggest that people on old MediaWiki versions can use old versions of this extension, and that new ones need only support the currently supported MediaWiki versions (although, I'm not sure we need to break backwards compatibility to add Gadget support, so this is a moot point).

And also, I don't think we need to get rid of the href on the purge link (but maybe that's just your testing, as you say).

Sorry if I'm just confused!! :-) Thanks for working on this stuff.

@Hutchy68
Copy link
Owner

Hutchy68 commented Nov 8, 2017

These changes are what I needed to get the js to load. getOutput in Master will not load the module. I thought getOutput was really only for Special pages anyway unless that changed. This is why I added the onBefore hook. With that hook the js will load as I took a screenshot in dev console.

Do you have your the extension js loading accidentally in another method. User js, Skin js or common or in an enabled gadget? Been there, done that. Tested something in user css or js and forgot to delete it.

@samwilson
Copy link
Contributor Author

I haven't had any problems with it not loading the JS correctly, including with $wgGroupPermissions['*']['purge'] = false;. Perhaps you're seeing something strangely cached? No one else seems to be reporting a problem with the current operation of the extension.

Anyway, we'll get it sorted if need be, but I must say I'm coming around to thinking that a gadget isn't really the way to go with this at all. See my comment on #16.

However, a good interaction with the Gadgets extension would perhaps be a migration maintenance script that checks for a gadget-purge user preference and sets the purge preference accordingly. Does that sound like a better idea?

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

No branches or pull requests

2 participants