Page MenuHomePhabricator

calling custom javascript in mw.util.addPortletLink doesn't work anymore
Closed, DeclinedPublic

Description

In dewiktionary my gadget https://de.wiktionary.org/wiki/Benutzer:Formatierer/checkpage.js adds a PortletLink to call a javascript function. The relevant code is:

if (document.getElementById("ca-view")) {
     var cp = mw.util.addPortletLink("p-views", null, "Check", "ca-checkpage",
              "checkpage() für diese Seite aufrufen",null,"#ca-view");
     $(cp).click(_public.analyzeThisPage);
 }

Since 1.32.0-wmf.22 this does not work anymore. Instead by clicking the link the page https://de.wiktionary.org/wiki/null is loaded.

Event Timeline

Aklapper changed the task status from Open to Stalled.Sep 20 2018, 3:18 PM

Any debug output with debug=true? See https://www.mediawiki.org/wiki/Help:Locating_broken_scripts

What would be an example URL where to see the gadget in action, so someone else can try to reproduce the problem?

@Formatierer: Let's please keep the discussion centralized in one single place. Thanks. :)

On which page would I be supposed to see something? Any and every page I go to via https://de.wiktionary.org/wiki/Spezial:Zuf%C3%A4llige_Seite ?

The gadget has to be activated in Einstellungen/Helferlein CheckPage. Then in every entry in namespace 0 an extra menu entry "Check" will appear. Clicking on it causes the de.wiktionary.org/wiki/null page to load.

checkpage.jpg (314×1 px, 75 KB)

Thanks! So you end up at null because of the second parameter and because $(cp).click(_public.analyzeThisPage); is ignored.

https://www.mediawiki.org/wiki/ResourceLoader/Core_modules#addPortletLink says to "use the jQuery(...).click on the element returned by the addPortletLink function, to specify the code which should be executed" and https://doc.wikimedia.org/mediawiki-core/master/js/#!/api/mw.util-method-addPortletLink provides example code. Does that help?

I tried
var cp = mw.util.addPortletLink("p-views", "", "Check", "ca-checkpage", ...
and
var cp = mw.util.addPortletLink("p-views", window.location.href, "Check", "ca-checkpage", ...
In the first case the main page is loaded, in the last case the current page is reloaded, but the intended function call is not executed, when clicking the button.

This seems to be a side effect of rMWbe2774703aae: mediawiki.util: Optimise addPortletLink, which changed the function to set the href as a property rather than as an attribute. Previously passing null resulted in no href attribute, now it it cast to a string. When there was no href attribute, clicking the link did nothing, but now the browser will navigate to its target.

@Formatierer We'll probably fix it, but in the meantime, you can change your event handling function (_public.analyzeThisPage) to return false to prevent the browser's default action of navigating to the link's target.

mw.util.addPortletLink is documented as taking three required parameters of type "string". They are portletId, href, and text. It is not valid to set null here, a string must be used.

https://doc.wikimedia.org/mediawiki-core/master/js/#!/api/mw.util-method-addPortletLink

To create a link that "goes nowhere", as @matmarex points out, you should return false from the click handler, or call event.preventDefault(). The documentation (linked above) actually shows an example of how to do that. This way has existed for many years, and is the standard way in JavaScript code to tell the web browser that you want to disable the default behaviour of a link (default behaviour = going to an address or hash). See also https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault

The previous code worked by accident. It was not documented or encouraged. The code has now been improved and we have helped and resolve a problem in the code. I'll close the task as such.

The previous version of addPortletLink() also did not allow href as null, nor href null to remove attribute. This worked due to indirect compatibility behaviour triggered through jQuery that translates attr('href', null) to removeAttr('href'). I was willing to support this if the original pre-jQuery version of addPortletLink allowed that, but I found that is not the case. I have checked the code from 2010 and found that code behaves the same way as today. As such, there is not a risk of breaking compat with wide-spread code that used to work.

Sorry about the inconvenience, we always try to avoid breaking changes, and when we do, they should involve deprecation warnings and Tech-News announcements first. I hope the above helps to explain why this was not considered a breaking change, and I hope the above helps you solve the problem. Feel free to ask more questions here :)