This is an archive of past discussions about MediaWiki:Gadget-popups.js. Do not edit the contents of this page. If you wish to start a new discussion or revive an old one, please do so on the current talk page.
Issue if there is a DOM element with id attribute "pg"
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
Hello,
An user reported on the French wiki that this script is not executed if there is an element with id attribute "pg" in the page.
Indeed, there is an old JavaScript behaviour that exposes the elements with an id to properties of the window object. For instance see this article: DOM: element IDs are global variables, the comments are also worth reading. This behaviour is considered very quirky and kept only for backward compatibility, though it's barely used and its use is discouraged. So, the gadget Popups also uses a window.pg property and looks for it to avoid multiple executions, but as it encounters the one from the legacy JavaScript, the early return is triggered.
A fix would be:
/* Bail if the gadget/script is being loaded twice *//* An element with id "pg" would add a window.pg property, ignore such property */if(window.pg&&!(window.pginstanceofHTMLElement)){return;}/* Export to global context */window.pg=pg;
Note we are overwriting the legacy window.pg property, but I don't think it would cause trouble, and I suppose globally renaming the Popup's window.pg property would be a much more difficult task.
Question about possible recently deployed changes to Navigation Popups gadget
Historically, as in prior to this fall, the user group names that were displayed were the user group names as defined in the MediaWiki database tables (i.e., sysop, autopatrolled, etc.). More recently, though, the group names display, presumably, from the displayed text at MediaWiki:Group-sysop-member, as in that example. I'm wondering if there has been a recent change to the gadget that made this change and, if so, which change was it? Ordinarily, this would not be a big deal, but we many users use this very helpful gadget on non-Wikimedia projects (notably, Miraheze). As a result, if there was a recently deployed gadget change that caused this, since many Miraheze-hosted wikis use custom groups, they will not necessarily have a [[MediaWiki:Group-groupname-member]] interface message created, resulting in the displayed text in the popup displaying as {Group-member-member} in the navigation popup. Miraheze does use the MirahezeMagic extension that effectively provide for the ability to deploy global interface messages, which is generally done for the standard/default groups, but for the custom group names some wikis will create, it results in an undesirable display.
Assuming there was a recent change to this gadget, I'm wondering if it would, therefore, be possible to have the gadget display the actual groupname rather than the displayed wikitext from the interface message page? Dmehus (talk) 00:47, 14 November 2021 (UTC)
It looks like this was requested in this change. I'm wondering how married to the idea users are with using the displayed text version of the group names rather than the technical group names (as was done previously)? The other changes requested in that request all seem reasonable; it's just that one that I'm wondering if we could possibly revert? Dmehus (talk) 00:54, 14 November 2021 (UTC)
I just had another idea. Perhaps, if not wanting to revert to the technical group names, could we add an if/else statement to the gadget's code whereby if no interface message is defined, it uses the technical group name and, if there is an interface message defined, it uses the displayed text version of the group name? Dmehus (talk) 01:00, 14 November 2021 (UTC)
I liked the tech names better myself actually. As far as foreign projects not liking our local config - they can feel free to fork it to whatever they want. — xaosfluxTalk02:12, 14 November 2021 (UTC)
Thanks for the quick reply! I assume you meant you liked the localised group names better than the tech group names? As to forking the gadget, yeah, I was trying to avoid having to fork the gadget, so as to maintain and update only one codebase. What about the alternate solution, of having the gadget perform a check to see if a local interface message for the group name exists and, if no, then it uses the tech group name rather than the localised group name? Dmehus (talk) 17:04, 14 November 2021 (UTC)
I don’t think there’s a considerable time penalty in using mw.Message.exists() as it runs entirely on the client side. I think the following change could work (although I haven’t tested it):
As far as foreign projects not liking our local config - they can feel free to fork it to whatever they want.
Sorry, but this is the worst possible advice. Forks most likely won’t be updated not only with new features, but not even with fixes made necessary by changes in MediaWiki (or its extensions). This means that the forks will break sooner or later, causing bad user experience and log spam if client-side errors are logged (as is the case on Wikimedia wikis). A much better solution would be a configuration setting, which could be set on a site level with a much lower maintenance cost, and you could even easily set it for yourself to return to the technical names here on the English Wikipedia. —Tacsipacsi (talk) 20:42, 14 November 2021 (UTC)
Yeah, but we basically have no reason to care about non-WMF wikis, especially those hosted at another server farm which have their own support networks. That's besides the obvious security issues of pulling in JavaScript from a non-"local" network... Miraheze should be interested in blocking such pulls. Izno (talk) 23:49, 27 November 2021 (UTC)
XSS vulnerabilities fixed
Earlier on I fixed four separate XSS vulnerabilities in this gadget. The first three of them were due to the link text in the generalLink function not being escaped, which I fixed here, and the fourth is due to not removing user-supplied HTML before generating the box preview on the edit screen, which I fixed here. I also made a few edits escaping outputs that should be escaped, but weren't actually the cause of any vulnerabilities. If anyone notices any problems as a result of these edits, please let me know. All of the vulnerabilities required non-default options, so it is unlikely that many people were affected (especially as the options format seems to have changed, and the documentation doesn't seem to have been updated). Here are the details of each of them:
phab:T297602: If an attacker saves a page with redirect syntax containing a payload like #REDIRECT [[<img src="x" onerror="alert(1)" />]], then when the victim views a popup of that page the attacker's code is run. This requires the popupAppendRedirNavLinks option to be false (the default is true). I fixed this by escaping the link text in generalLink. To avoid breaking existing HTML entities I decoded them before encoding the string again.
(No Phabricator number) The attacker creates a page with a disambiguation template and a link containing an XSS payload, like [[<img/src="x"onerror="alert(1)"/>|Bar]]{{disambiguation}}. When the victim views a popup of this page, the attacker's code is run. This requires the popupFixDabs option to be true (default is false). The root cause is the same as #1 so I didn't file a Phabricator ticket for it.
(No Phabricator number) For this one, all the attacker has to do is create a link to a disambiguation page with an XSS payload in the section link, like [[Foo (disambiguation)#<img/src='x'onerror='alert(1)'/>]]. When the victim hovers over the link, the attacker's code is run. This also requires the popupFixDabs option to be true, and also has the same root cause as #1.
phab:T297560: The attacker saves a page containing a link followed by an XSS payload, like [[Foo]]<img src="x" onerror="alert(1)" />. If the victim edits that page, selects the link and the payload in the edit window, and hovers the mouse over the selection, the attacker's code is run. This requires the popupOnEditSelection option to be set to "boxpreview" (the default is "cursor"). I fixed this by using the same preview-generation code as used elsewhere in the gadget so that user-supplied HTML tags would be stripped.
These four vulnerabilities are the result of an exhaustive analysis of all the possible vulnerable code paths, which took me a couple of weeks on-and-off. If there are any XSS bugs still left in the gadget, then they are not obvious ones. (There are probably still issues with older IE versions, but those are not supported by Mediawiki any more.) In order to avoid issues like this in the future, this gadget could probably use a large-scale refactor to use safer coding idioms, but for now at least there should be no glaring security holes. — Mr. Stradivarius♪ talk ♪15:06, 27 December 2021 (UTC)
Edit request for popupDabRegexp
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
The current regex does not catch markup like {{dab|geodis}}, as you can see by trying to dabfix this link with Popus: Anjou. The whole regex seems a bit too complicated as it is, so I propose a simpler one, which AFAICT will match all the same things as the current version, plus this false negative.
\{\{\s*(d(ab|isamb(ig(uation)?)?)|(((geo|hn|road?|school|number)dis)|[234][lc][acw]|(road|ship)index))\s*(\|[^}]*)?\}\}|is a .*disambiguation.*page
Assuming there's no issues with this regex, then please change line 7112 to
newOption('popupDabRegexp','\\{\\{\\s*(d(ab|isamb(ig(uation)?)?)|(((geo|hn|road?|school|number)dis)|[234][lc][acw]|(road|ship)index))\\s*(\\|[^}]*)?\\}\\}|is a .*disambiguation.*page');
Thank you for the request - I've updated the gadget with your regex. Looking at Template:Disambiguation#Variant templates, there are a lot of disambiguation templates that are missed by this regex (and the old one), but this is definitely an improvement. Also, from JavaScript we should be able to use the API to check if the page is a disambiguation page or not without having to parse its wikitext, but that would require larger-scale changes to the code, so this is a good stop-gap solution. — Mr. Stradivarius♪ talk ♪03:04, 28 December 2021 (UTC)
This file has a mix of styles for indentations and spacing, mostly being to avoid spaces and newlines as much as possible, which makes it really difficult to read through and make changes. To make it easier, I suggest we use a tool like prettier for a one-time reformatting to clean code. – SD0001 (talk) 07:03, 25 November 2021 (UTC)
SD0001, if you could send me a pastebin or a raw text file on Discord or by email with what you'd like, I'd be happy to swap it. I looked at the options and was generally bewildered. :) Izno (talk) 01:11, 4 January 2022 (UTC)
Do you just want to do something like this - adding 70000 bytes of whitespace to the source? I really don't want to start seeing white-space wars in javascript files - and especially don't want to hear tab-vs-spaces fights. — xaosfluxTalk02:06, 4 January 2022 (UTC)
I would generally support using tabs, since that's the default in CodeEditor these days, but beyond that, I have no strong preference. (And I obviously don't work in JavaScript.) Izno (talk) 01:35, 5 January 2022 (UTC)
I support any changes to improve formatting; the existing poor formatting has certainly slowed me down while reading and changing the script. No preference on tabs vs spaces (and a million programmers howled in the distance). Enterprisey (talk!) 06:26, 5 January 2022 (UTC)
Note, I'm not opposed to a format update, but don't want to see some format warriors going through scripts that they don't actually maintain themselves to try to enforce their own layout style or start a local indentation holy war! So, that being said - if someone wants an udpate, fork this to your personal sandbox, update it - then drop us an ER :) — xaosfluxTalk14:40, 5 January 2022 (UTC)
I wouldn't mind spaces or double quotes, it just gotta be consistent. I suggest tabs and single only because they already are used in the majority of the code (as well as in MW code). Nardog (talk) 18:47, 5 January 2022 (UTC)
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
Sanity testing can be done by disabling navpopups and popups, then running in console: importScript('User:SD0001/popups-prettified.js'); importStyleSheet('MediaWiki:Gadget-navpop.css')Diff will of course be useless. But it can be verified that no functional changes are made by running
# nodejs and npm should be installed. (Easy way: go to play-with-docker.com and run `apk add nodejs npm`)
curl-shttps://en.wikipedia.org/wiki/MediaWiki:Gadget-popups.js?action=raw>popups.js
curl-shttps://en.wikipedia.org/wiki/User:SD0001/popups-prettified.js?action=raw>popups-prettified.js
npxprettier--use-tabs--single-quote--print-width100-wpopups.js
diffpopups.jspopups-prettified.js
wikEdDiff eats the diff for breakfast. All I see are whitespace improvements and one or two 'normalized' regexes. Izno (talk) 17:47, 6 January 2022 (UTC)
Prettier is formatting the docstring comments a little awkwardly. For example, take lines 672-682:
/** Creates a new Drag object. This is used to make various DOM elements draggable. @constructor*/functionDrag(){/** Condition to determine whether or not to drag. This function should take one parameter, an Event. To disable this, set it to <code>null</code>. @type Function */this.startCondition=null;
It looks like Prettier is trying to preserve the original indentation of the comments, so we get things like the text in the second comment (line 678) being prefixed with a tab and three spaces. This is the same as the original file (line 571) but we probably want to edit these so that they are lined up properly and use only tabs. I'll take a look at this. — Mr. Stradivarius♪ talk ♪14:30, 7 January 2022 (UTC)
Yeah I saw that. The existing doc-comments were of a non-standard format, so it's a GIGO issue. Feel free to switch them over to the standard
I've got the following error when trying to patrol a page (on Russian wiki):
load.php?modules=ext.gadget.Navigation_popups:133 Uncaught ReferenceError: api is not defined
at HTMLAnchorElement.a.onclick (load.php?modules=ext.gadget.Navigation_popups:133:197)
Maybe it is not an issue on enwiki, but on some older sister projects (eg. metawiki, zhwiki), a lot of users registered before 2006 or something does not have the value for the user_registration field (an example), and since the popup gadget associates registration date with edits, their edits count just don't show up at all:
As user.registration is not necessary for older users to have actual edits, I wonder if changing the conditionals, for example using user.editcount, or (either user.registration or user.editcount) as the condition, is possible to fix that? —— Eric Liu（Talk）03:42, 17 May 2022 (UTC)
Display equivalent QID
Display and link the QID (in this case Q144), inside unlinked parentheses
I've been asking for some time for the popup to display the equivalent QID (i.e. the Wikidata identifier) when used on pages which have one. This will save me (and no doubt others) time and mouse clicks every single day. I've mocked up what I mean in the above image, and put more details on Phabricator, in T243820.
This happened when I unwatched a page via the popup's interface. I didn't scrutinize the source code at all but probably there's no variable substitution for api in the function? Developers might want to check it out. --Dragoniez (talk)11:52, 8 February 2023 (UTC)
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
This uses the api object fetcher, and removes the loader.using statement for mediawiki.api because that dependency is already enforced at line 7013. —TheDJ (talk • contribs) 14:24, 8 February 2023 (UTC)
Hmm, getting a parsing error complaining about a missing parenthetical that for some reason I can't find. Self-reverted for now. LMK if you can see what I missed. (Also, sidenote: I don't think we can include the "return" or semicolon, since this isn't a standalone line of code, but an argument to a when() function.) Writ Keeper⚇♔15:23, 8 February 2023 (UTC)
@TheDJ:, @Writ Keeper: Thank you both. I managed to find some time to look into this, and this issue (per se) can be resolved if we replace the whole API call lines as below:
But there's another problem, I looked through the code but loadMessagesIfMissing isn't defined anywhere in the script. This must be why Writ Keeper got a parsing error. But, this function is called in line 5110 too.
I'm pretty sure we'll need time if we try to fix this too, so could we just temporarily resolve the current issue and look into this one later? I currently don't even know what kind of error this one would spit. Thanks. --Dragoniez (talk)15:41, 8 February 2023 (UTC)
No, loadMessagesIfMissing is a function of the mw.Api object, which is Mediawiki-defined and returned by the getMwApi() function. You can find its definition here: https://doc.wikimedia.org/mediawiki-core/master/js/#!/api/mw.Api.plugin.messages. I suspect the parsing issue I was seeing was just related to the above-mentioned problem with including the return ...; that was persisting unusually strongly through cache refreshes, since it seems to be working now. I'll reinstate the change shortly, if additional testing bears out. Writ Keeper⚇♔15:50, 8 February 2023 (UTC)
@Writ Keeper: Sorry I guess I looked at the code too quickly. Shoud've noticed that because getMwApi() initializes a mw.Api() instance. The following worked for me without any error in console:
I noticed today that using popups to unwatch a page from my watchlist once again shows the brief notification pop-up. That hadn't happened for some considerable time, now it's fixed. Thank you. -- Michael Bednarek (talk) 00:56, 9 February 2023 (UTC)
I'm seeing around 100 errors a day with the following stack trace on diff pages e.g.https://en.wikipedia.org/w/index.php?diff=1139175290&oldid=1139164047&title=Emily_Ratajkowski
:
at Title.namespaceId https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:49:957 at isInStrippableNamespace https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:53:921 at navlinkTag.getPrintFunction https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:127:213 at navlinkTag.html https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:123:239 at navlinkStringToHTML https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:123:111 at pg.structures.menus.popupTopLinks https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:23:605 at pg.structures.shortmenus.popupTopLinks https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:23:988 at fillEmptySpans https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:64:38 at simplePopupContent https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:6:915 at mouseOverWikiLink2 https://en.wikipedia.org/w/load.php?lang=en&modules=ext.gadget.Navigation_popups&skin=monobook&version=nxu22:6:90
My request is to add the Yiddish redirect code to the redirLists. That is, after line 5679 in the source ("vi: [r, 'đổi'],"), please add the following line:
This should be a change to the way links are rendered so clicking on the actions and popups menu and making sure the individual entries behave the same should work. Sohom (talk) 20:24, 7 December 2023 (UTC)
This patch has some minor linter errors, but I'll clean that up in my next linter autofixes patch. Y
{bool} should be {boolean}, I think. N
What does UNSAFE START and UNSAFE END mean? HTML sanitization related, I assume? Consider elaborating on the comment. N
What is the difference between link.text and link.title? Consider elaborating in the comment. N
Looks like this returns null sometimes? Should probably change to @returns {string|null} N
It appears that the Show recent diff module was sending incorrect API params to the API, causing the diff display to fail, the changes below should allow it to be shown again :)
@Novem Linguae Hover over a link, and mousing over the actions link, and then mouse over to the "Show recent edit" link. If the patch is not applied, it will show the header of the page, if it applied it will show the diff of the most recent edit. Sohom (talk) 21:37, 4 December 2023 (UTC)
@Sohom Datta I was porting this to plwiki and noticed `fromtitle` don't seem to support sections. Am I correct? If so would probably be good to skip an anchor:
@Nux Can you reproduce a case where this occurs ? I did test with URL fragments (anchors) but I wasn't able to reproduce any issue. My understanding is that the anchor gets removed globally on line 7453 (quoted below), so it doesn't really matter if we omit anchors or not.
:// FIXME anchor handling should be done differently with Title object:this.article=this.article.removeAnchor();:
Yeah, no. I just looked at the `Title` class. I was even wondering if sections are at all possible in history. So maybe it doesn't matter. Nux (talk) 23:45, 4 December 2023 (UTC)
This adds braces to every if/elseif/else/for/while/do/switch, and fixes tabs/spacing. I did not run any other ESLint rules yet.
The author's original style was to often omit braces and/or put them all on one line. I think most would agree that this decreases readability and can lead to errors. I ran ESLint's auto fix feature, which should be really low risk since it's automated. No opportunity for a human to make a typo. I've tested the changes using the below code in my common.js:
I'll give the above changes a few days to make sure they didn't break anything, then I plan on making the below changes. Please let me know if you have any feedback.
The popupDabRegexp as it currently stands doesn't capture some disambiguation templates, e.g. {{Place name disambiguation}} and presumably all templates ending in "disambiguation}}". This was raised at Wikipedia talk:Tools/Navigation popups#No dab links when using {{Place name disambiguation}}. After an analysis by User:Tacsipacsi, popupDabRegexp was identified as the culprit. I then added a term to that expression that will capture such category names. I used that modified expression in my popups configuration, and that successfully resolved the problem. I think it would be an improvement if this were the default as coded overleaf. Please change the string after newOption('popupDabRegexp', (line 9069), to
popupDabRegexp='disambiguation\\}\\}|\\{\\{\\s*(d(ab|isamb(ig(uation)?)?)|disambiguation\\}\\}|(((geo|hn|road?|school|number)dis)|[234][lc][acw]|(road|ship)index))\\s*(\\|[^}]*)?\\}\\}|is a .*disambiguation.*page';
@Novem Linguae:Function and Array are object types and written capitalized. (Although the latter should rarely be used, as generally the element type should be indicated, producing string[], number[], any[] etc. instead of Array.) Only string, null, undefined, unknown, any and primitive types are written lower-case. —Tacsipacsi (talk) 22:00, 15 August 2023 (UTC)
Fixed. Thanks for the code review. I am disappointed in my IDE's linters for allowing me to type that and not putting a little red squiggle under it. –Novem Linguae (talk) 23:52, 15 August 2023 (UTC)
FYI, letf=function(){};typeoff; in Chrome DevTools console returns "function" not "Function". typeof['test','test'] returns "object" (an "Array" I assume). –Novem Linguae (talk) 13:05, 7 September 2023 (UTC)
Add QID to info line
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
Please add changes to CSS and JS to make menus in popups more stable (mouse out UX aka safe triangle technique).
Two changes:
Minor changes in JS JS diff (basically adds a CSS variable).
The extra CSS (CSS diff) creates a trapezoid that gives a safe path from the menu title to the list of links. There are also additional margins that will facilitate e.g. hitting the unfollow link.
CSS works without JS changes, but JS makes width of the trapezoid more stable (especially after i18n/l10n). It was my intention to do as much in CSS as possible. You can see the safe triangle technique in a Tweet.
Are you the sole maintainer for this gadget? If not, may want to add some more comments to this code so folks know what's going on. I don't think it's very easy to tell what's going on with that block of CSS just by reading the code. I think a good spot for a big comment that describes how this whole system works ("safe triangle technique") is at the top of the CSS code you're adding. Hope this helps. –Novem Linguae (talk) 04:58, 17 June 2023 (UTC)
@Novem Linguae well in practice nobody else is changing popups much... But I did add some comments see: pl:Gadget-Popups.css. As written there I'm using a trapezoid guide for the cursor, other CSS rules also got their comments. I haven't changed the old CSS (nor old JS) if that what you are referring to. Popups is generally not well documented (probably to save bytes, which used to matter ~10 years ago). Nux (talk) 09:03, 17 June 2023 (UTC)
Novem Linguae, Nux - Looking through the code, the JS change is essentially a variable name change with some small code optimization. The CSS looks like just a few small changes to the resulting function of the menus within the pop-up. I... don't see a reason to oppose the changes - Done. ~Oshwah~(talk)(contribs)02:41, 25 July 2023 (UTC)
@TheDJ I only had a moment for a quick check, but I think there's an issue accessing a preview of "history" from within the action menu of a popup? I'll try to confirm later, but wanted to put it out there; I presume an issue parsing rvslots stuff. ~ Amory(u • t • c)12:50, 12 April 2023 (UTC)
The current implementation of the parser has a minor bug where if there are formatting marks within the code of a link, that code gets converted to html and then escaped, so that the html code is displayed as text. The text [[Law & Order franchise|''Law & Order'' franchise]] displays as <em>Law & Order</em> franchise instead of Law & Order franchise.
I fixed the issue by having the code parse wiki formatting after the html is escaped instead of before. This way, the formatting marks are converted to html after the html is escaped, rendering the preview as intended. I posted the code at User:Shardsofmetal/popups.js. This diff shows the changes I've made. I've posted screenshots of before and after the change, with a red box around the bug. Could somebody update the file with this change? Thank you, Shardsofmetal00:32, 20 March 2022 (UTC)
I'm reactivating my request. I've updated the page at User:Shardsofmetal/popups.js to make this change to the current version of the script (diff between my proposed code and the current code). The last time I made this request, I didn't realize how confusing the diff page was; it made it appear as if I had made many more modifications to the code than I actually had. This time, I've made sure the diff page clearly shows how few modifications I've actually made. I'll restate my proposal because I don't think I was clear enough and detailed enough last time.
Current behavior: The script converts wikitext formatting for italic and bold text (''italic'' and '''bold''') to html (<em>italic</em> and <strong>bold</strong>) before converting wikitext formatting for links into html. When the script converts wikitext formatting for links, it escapes or strips out any html from the article source (for safety reasons). Unfortunately, this means that it's also escaping the html code that the script itself just put there, resulting in the behavior described above and demonstrated in the sreenshot.
Intended behavior: The script escapes or strips html from the article source text (to prevent potential XSS attacks), but does not escape or strip html that the script itself created when parsing wikitext (e.g. converting italic markup to italicized text).
Confirmation that html is still escaped or stripped from linksMy proposed change: Converts the wikitext formatting for italic and bold text after converting wikitext formatting for links and escaping or stripping html. This way, the <em></em> and <strong></strong> tags created by the script won't get escaped, while any html in the article source text is still escaped or stripped. (To demonstrate this, I created a test page at User:Shardsofmetal/popupsLinkTest. The page contains 2 links to the "Wikipedia" article with markup to italicize "Wikipedia". The first link uses wikitext and the second uses html. If you're running the version of popups with my proposed change and hover over the link to my test page, you can see that the html is still stripped out of the second link. I've attached a screenshot.)
This proposed change does not introduce any XSS vulnerabilities. None of the code related to escaping html has been modified.
Hopefully this explanation was clear and detailed. Please let me know if there are still any questions or concerns, or if there is anything else I can do get this implemented. Thank you — Shardsofmetal [talk] 00:20, 18 April 2023 (UTC)
This seems safe to me from an XSS perspective since the only HTML allowed is formatting generated by popups but will give this edit a bit for any comments otherwise. Galobtter (talk) 00:48, 18 April 2023 (UTC)
I see some hints in the code that this gadget has a repo somewhere and is compiled with a deploy script. For example // ENDFILE: run.js. Any idea where the repo is? –Novem Linguae (talk) 10:36, 15 August 2023 (UTC)
List of redirect magic words for each language could use refactoring
I have no idea why the code in #Turkish Redirect above randomly sprinkles 'r' for 'redirect' or 'R' for 'REDIRECT' into every row. My testing indicates both these keywords work on Turkish Wikipedia. A potential future patch could be to refactor the code to be something like [r, R, ...] on every line. –Novem Linguae (talk) 09:51, 15 August 2023 (UTC)
// Add Popups to an anchor Element; popData is optional.
// Note that the link should be a wiki link.
function addTooltip(a, popData) {
if (!isPopupLink(a)) {
return;
}
a.onmouseover = mouseOverWikiLink;
a.onmouseout = mouseOutWikiLink;
a.onmousedown = killPopup;
a.hasPopup = true;
a.popData = popData;
}
pg.fn.addTooltip = addTooltip;
Not done for now:Nux why do you think this is something necessary? Can you provide some actual use cases or proposed changes? It might be more valuable simply to add whatever it is you think should be added to this script rather than extended. Izno (talk) 19:43, 3 June 2022 (UTC)
@Izno I'm using this on plwiki in custom menu, so I doubt that could be done in a different way then by exposing `addTooltip` function.
@Izno Note that view and history tabs is now in main content (in V22) so it will get popups anyway. This probably was not intentional... but it work for me, so I'm not complaining 🙂
The use case for custom, dynamic links still remain. Still could use `pg.fn.addTooltip`.
dismabig fixer (arrows are links) Here's another use case from the Polish Wikipedia, a gadget that corrects links to disambiguation pages (disFixer tool). The disFixer tool is widely used and originally created by MatmaRex (currently a developer of MW/WMF). This gadget could easily be ported to the English Wikipedia, but the changes to the original Popups are missing. As of this year, disFixer uses the Popups fork to improve links [3]. Since disFixer creates links dynamically, you can't really handle this on the Popups side. Similarly, other gadgets would need a hook (to know that the Popups is ready) and a function to enhance the selected links they create. Nux (talk) 16:52, 25 March 2023 (UTC)
Still missing this on en.wiki. Any chance to add this here? Is there a technical issue or something I can help with? This has been running on pl.wiki for over a year now. --Nux (talk) 23:53, 10 June 2023 (UTC)
Not done this request is stale, and multiple other changes have been implemented that would have forked the version. Also, it doesn't appear to be uncontroversial. If you wish to proceed please continue the discussion and make a sandbox of the current version and your proposed diff again (you can just put it at Special:MyPage/Gadget-popups-sandbox.js for example. — xaosfluxTalk03:15, 8 October 2023 (UTC)
Turkish Redirect
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
Can you please add "YÖNLENDİRME", "yönlendirme", "YÖNLENDİR" and "yönlendir" as Turkish translations of redirect at line 5685? Thanks --ToprakM✉17:04, 30 July 2023 (UTC)
I didn’t ask why they have been hardcoded, but why they are hardcoded, now, in 2023, well over a decade after the creation of the action API. Why new and new aliases get hardcoded instead of rewriting it to use the API and making it work in any language. —Tacsipacsi (talk) 21:55, 15 August 2023 (UTC)
Your approach sounds like the "correct" fix, but would require more coding effort/brainpower than just adding a row to some JSON. It most likely boils down to a lack of volunteer time/effort to refactor that section of code. I would happily review an {{IAER}} using that approach though since I've got that section of code top of mind right now. –Novem Linguae (talk) 23:58, 15 August 2023 (UTC)
@@ -5662,56 +5662,6 @@ $(function () { pg.nsTemplateId = 10;
}
- function setRedirs() {- var r = 'redirect';- var R = 'REDIRECT';- var redirLists = {- //<NOLITE>- ar: [R, 'تحويل'],- be: [r, 'перанакіраваньне'],- bg: [r, 'пренасочване', 'виж'],- bs: [r, 'Preusmjeri', 'preusmjeri', 'PREUSMJERI'],- bn: [R, 'পুনর্নির্দেশ'],- cs: [R, 'PŘESMĚRUJ'],- cy: [r, 'ail-cyfeirio'],- de: [R, 'WEITERLEITUNG'],- el: [R, 'ΑΝΑΚΑΤΕΥΘΥΝΣΗ'],- eo: [R, 'ALIDIREKTU', 'ALIDIREKTI'],- es: [R, 'REDIRECCIÓN'],- et: [r, 'suuna'],- ga: [r, 'athsheoladh'],- gl: [r, 'REDIRECCIÓN', 'REDIRECIONAMENTO'],- he: [R, 'הפניה'],- hu: [R, 'ÁTIRÁNYÍTÁS'],- is: [r, 'tilvísun', 'TILVÍSUN'],- it: [R, 'RINVIA', 'Rinvia'],- ja: [R, '転送'],- mk: [r, 'пренасочување', 'види'],- nds: [r, 'wiederleiden'],- 'nds-nl': [R, 'DEURVERWIEZING', 'DUURVERWIEZING'],- nl: [R, 'DOORVERWIJZING'],- nn: [r, 'omdiriger'],- pl: [R, 'PATRZ', 'PRZEKIERUJ', 'TAM'],- pt: [R, 'redir'],- ru: [R, 'ПЕРЕНАПРАВЛЕНИЕ', 'ПЕРЕНАПР'],- sk: [r, 'presmeruj'],- sr: [r, 'Преусмери', 'преусмери', 'ПРЕУСМЕРИ', 'Preusmeri', 'preusmeri', 'PREUSMERI'],- tr: [R, 'YÖNLENDİRME', 'yönlendirme', 'YÖNLENDİR', 'yönlendir'],- tt: [R, 'yünältü', 'перенаправление', 'перенапр'],- uk: [R, 'ПЕРЕНАПРАВЛЕННЯ', 'ПЕРЕНАПР'],- vi: [r, 'đổi'],- yi: [R, 'ווייטערפירן'],- zh: [R, '重定向'], // no comma- //</NOLITE>- };- var redirList = redirLists[pg.wiki.lang] || [r, R];- // Mediawiki is very tolerant about what comes after the #redirect at the start- pg.re.redirect = RegExp(- '^\\s*[#](' + redirList.join('|') + ').*?\\[{2}([^\\|\\]]*)(|[^\\]]*)?\\]{2}\\s*(.*)',- 'i'- );- }- function setInterwiki() {
if (pg.wiki.wikimedia) {
// From https://meta.wikimedia.org/wiki/List_of_Wikipedias
@@ -6815,20 +6765,24 @@ $(function () { }
}
- function fetchSpecialPageNames() {+ /**+ * Fetch site information.+ * @returns {Promise}+ */+ function fetchSiteInfo() { var params = {
action: 'query',
meta: 'siteinfo',
- siprop: 'specialpagealiases',+ siprop: ['specialpagealiases', 'magicwords'], formatversion: 2,
- // cache for an hour uselang: 'content',
+ // cache for an hour maxage: 3600,
};
return getMwApi()
.get(params)
.then(function (data) {
- pg.wiki.specialpagealiases = data.query.specialpagealiases;+ pg.wiki.siteinfo = data.query; });
}
@@ -6890,7 +6844,7 @@ $(function () { var sp = nsRe(pg.nsSpecialId);
pg.re.urlNoPopup = RegExp('((title=|/)' + sp + '(?:%3A|:)|section=[0-9]|^#$)');
- pg.wiki.specialpagealiases.forEach(function (specialpage) {+ pg.wiki.siteinfo.specialpagealiases.forEach(function (specialpage) { if (specialpage.realname === 'Contributions') {
pg.re.contribs = RegExp(
'(title=|/)' +
@@ -6932,6 +6886,16 @@ $(function () { );
}
});
+ pg.wiki.siteinfo.magicwords.forEach(function (magicword) {+ if (magicword.name === 'redirect') {+ var aliases = magicword.aliases.map(function (alias) { return mw.util.escapeRegExp(alias); });+ // Mediawiki is very tolerant about what comes after the #redirect at the start+ pg.re.redirect = new RegExp(+ '^\\s*(' + aliases.join('|') + ').*?\\[{2}([^\\|\\]]*)(|[^\\]]*)?\\]{2}\\s*(.*)',+ 'i'+ );+ }+ }) //<NOLITE>
var im = nsReImage();
@@ -7039,7 +7003,7 @@ $(function () { 'user.options',
'mediawiki.jqueryMsg',
])
- .then(fetchSpecialPageNames)+ .then(fetchSiteInfo) .then(function () {
// NB translatable strings should be set up first (strings.js)
// basics
@@ -7055,7 +7019,6 @@ $(function () { // regexps
setRegexps();
- setRedirs(); // other stuff
setMisc();
Maybe we could even remove pg.wiki.lang as it seems to become unused with this change (hooray, it probably means this was the last hardcoded content language-specific thing!), but I’m not confident enough to do it. Tested on my local, Hungarian-language wiki, with both English and Hungarian redirects. —Tacsipacsi (talk) 18:56, 17 August 2023 (UTC)
It seems to be used only for formatting time. So it would probably only help in optimization of the history page preview (if at all significant). Not sure if that is something worth optimizing. You know "premature optimization..." etc Nux (talk) 22:51, 24 September 2023 (UTC)
Refactoring when you loose compatibly, do a lot of work and gain nothing in practice is a bad idea IMHO. When you would start refactoring that you would never finish ;). When you go up the stack you also have a custom `map` function. Nux (talk) 11:50, 25 September 2023 (UTC)
If both behave identically, using internal functions is always better than writing your own. 1) More standardized so more readable. 2) Less lines of code. 3) More performant. –Novem Linguae (talk) 19:33, 1 December 2023 (UTC)
To be a bit more constructive here... I think a better refactoring effort might be to be to move Popups to Github (e.g. with Wikipedia:Wiki-to-Git). Then do a split to separate files. And then either use e.g. browserify to combine it again. I've been thinking abut this for a while for plwiki, not sure if such update would be accepted here. Nux (talk) 12:19, 25 September 2023 (UTC)
I'd be open to this if this page gets more activity. With current activity levels though, moving to GitHub may just result in the GitHub being neglected and the versions getting out of sync. And also adds deploy complexity. https://github.com/wikimedia-gadgets/ would be a good spot for this if popups development gets more active (more folks working on this would justify the added complexity of an off-site repo). –Novem Linguae (talk) 19:36, 1 December 2023 (UTC)
Not done this doesn't appear to be a ready-to-go release, and may need additional discussion as well. Please prepare a current version to proposed version in a sandbox, such as at Special:MyPage/Gadget-popups-sandbox.js for the proposed code. IF this section is just to discuss strategies and proposals of course you may continue the discussion below. Best regards, — xaosfluxTalk03:17, 8 October 2023 (UTC)
Live preview refactor
@Novem Linguae I've done another round of refactoring and have ended up with this diff. The salient highlights are:
Removed the custom wikitext parsing engine at wiki2html() and replaced it a call to the parsing API
Removed defaultPopupsContainer() and replaced it with a lookup to #mw-content-text (since that is the stable way to identify where the content starts)
Refactored the generateLink() to not use raw HTML manipulation
You shouldn't remove `defaultPopupsContainer`. You should add new selector on top of the selectors, not remove fallbacks. You've also removed `popupOnlyArticleLinks` options by removing `defaultPopupsContainer`. I use popus on e.g. history tab. Nux (talk) 23:28, 5 December 2023 (UTC)
I've added back the function in my version, the updated diff should be this (lmk if you find any other issues). That being said, I did test the refactoring on quite a few cases (including the history tab, edit and preview usecases) and did not find any noticable difference in behaviour with #mw-content-text. Sohom (talk) 23:45, 5 December 2023 (UTC)
Thanks. I like the idea behind the changes. So I hope you don't mind me looking over your shoulder, but this is a popular gadget, which I'm also using :). Some thoughts upon review:
`.bind( this )` is not needed most of the time. The `this` keyword is not used inside of most calls to `makePreview`.
`APIimagepagePreviewHTML` doesn't have the same flow. At least looking at the code after `if (typeof content === 'string')` is another if (not else if). So `return ret;` below will not be correct (probably, quite likely).
`APIsharedImagePagePreviewHTML` is in `pg.fn`, so it is exposed to other gadgets and user scripts (possibly scripts on meta.wiki). Not sure if you can just change it from a synchronous function to async. I would at least expose the Promise.
This is mostly nit picking, but I think `// STARTFILE: livepreview.js` should be preserved. I still hope some bold dev, with admin rights, will one day cut the code to files ;).
In any case, I think large changes like this should go through beta process. Even if the code would mostly be equivalent, there might be some cases that are not easy to test. Nux (talk) 00:10, 6 December 2023 (UTC)
@Sohom Datta I also noticed I think you've missed how htmlGenerator is used. Seems like showAPIPreview shouldn't work. Seems to expect non-async function. Nux (talk) 00:34, 6 December 2023 (UTC)
@Nux No issues, in fact the more issues we find the better :)
I've gone ahead and fixed the issue with APIimagepagePreviewHTML (lmk if there are any issues)
I did a global search for APIsharedImagePagePreviewHTML and the only instances are popup clones, so I'm assuming this should be fine.
I've added // STARTFILE: livepreview.js though, only the wrapper wiki2html function remains.
I've removed the unnecessary .bind( this ) calls :)
Regarding the beta process, I'm happy to wait a while for y'all to test and review the code. I'll put up a post at WP:VPT and WP:IANB to solicit more beta testers/people who might want to review the code :) Sohom (talk) 00:56, 6 December 2023 (UTC)
For anyone willing to betatest this, please disable the gadget in your settings and add the following to your common.js
Very nice work. Getting rid of the 769 lines of InstaView code (which is some of the smellier code in the codebase, with one letter variable names, for (; remain(); ), etc.) is definitely a maintainability win.
May I suggest splitting this into three patches? Want to diff out the "Removed the custom wikitext parsing engine at wiki2html() and replaced it a call to the parsing API" in a new section and tag it with {{IAER}}? Smaller patches spaced out over time means easier testing, and easier to tell what caused bugs if this talk page starts getting bug reports. –Novem Linguae (talk) 07:43, 6 December 2023 (UTC)
@TheDJ Section previews appears to work similarly to how it worked previously. User:TheDJ/common.css does fail, but it does so on the old navpopups code as well (we should fix that in a later request?). Barack Obama seems fine since Navpopups truncates the preview to only show a small portion of the wikitext before it reaches the preview code. There is some additional latency since we are moving to a API call from a synchronous parsing of HTML, but overall it should be a performance win if we are parsing long peices of wikitext since we are keeping the main thread free instead of busy parsing multiple bytes of wikitext :) Sohom (talk) 10:48, 6 December 2023 (UTC)
Article title hyperlinks in previews are black, not blue
Steps to reproduce: 1) vector 2010, not tested on other skins, 2) turn off navigation popups in Special:Preferences gadgets, 3) add the below code to User:abc/common.js, 4) hover over a valid article. What happens? Article title link is black. What should happen instead? Article title link is blue.
Looking at the code for .popup_mainlink a { color: #000; }, I think the devs intend for it to be black everywhere.
In the situation above, there are two CSS styles competing with each other. In one situation, .popup_mainlink a { color: #000; } wins out over the other styles. In the other situation, a:link, .mw-parser-output a.extiw, .mw-parser-output a.external, .vector-menu-portal .vector-menu-content li a { color: rgb(0,0,238); } wins out over the other styles.
Anyway, I think we should pick one color, then get it to be used consistently. Whether it's blue or black. I'm leaning blue.
I'm also curious why loading it as a non-gadget changes the CSS order of precedence, even though the two sets of selectors are identical. Perhaps there's a bug in the load code above? –Novem Linguae (talk) 20:37, 7 December 2023 (UTC)
MediaWiki:Gadget-navpop.css should be renamed to MediaWiki:Gadget-popups.css
This gadget uses a second file called MediaWiki:Gadget-navpop.css. Idea: Shouldn't have to look at the gadgets definition file to figure out what this gadget's CSS file is named. It'd probably make more sense for all files related to this gadget to start with MediaWiki:Gadget-popups. So I propose renaming MediaWiki:Gadget-navpop.css to MediaWiki:Gadget-popups.css –Novem Linguae (talk) 10:34, 15 August 2023 (UTC)
@Novem Linguae: Apart from it not working, yes. 🙂 You try to put JavaScript in a CSS file, which of course won’t work, but a working solution along these lines would be acceptable. Please also make sure to use absolute URLs because many – probably even most – affected imports are not on the English Wikipedia (importStylesheet uses a URL relative to the wiki it’s executed on, not to the wiki it’s defined on, which would be wrong). —Tacsipacsi (talk) 15:25, 8 December 2023 (UTC)
I would add copyright to pros. Most of the time, I see patches applied, admins don't use the name of the person who authored the patch. Most of the time, patches are small, but for bigger patches, that might be copyvio. I assume when exporting one would use Wikipedia:Wiki-to-Git or similar to preserve authors :) (at least authors applying patches). And then when you do pull requests the authors are automatically preserved.
Also to pros I would add blame. Use that very often to figure when and why particular code was added. Making blame work after splitting to files might be a bit hard on git (no built in copy function). But there are ways to do that with a bit of branch and merge magic. Nux (talk) 10:35, 6 December 2023 (UTC)
I would definitely support a repository. Amongst other things, it will allow for much better code review. Another nice feature would be the ability to migrate to a sane linting system and potentially migrating to using the ResourceLoader package file system in the future (instead of one big huge 8000 line file).
I don't have any issues with eithier Wikimedia Gitlab or Github (as long as Gerrit is not considered), though it might be worthwhile to ask for a specific repos/gadgets namespace in Gitlab if we want to do it longterm :) Sohom (talk) 11:35, 6 December 2023 (UTC)
@Michael Bednarek I don't think there are any visibility issues as such, the code needs to be onwiki and in plaintext for the gadget to be able to run at all regardless of whether we use a single huge file or multiple small files with the packagefile directive. Regarding the code review process, it would happen primarily on the platform Gitlab/Github similar to how the Twinkle code review model currently works. Sohom (talk) 14:47, 6 December 2023 (UTC)
While code review would happen on GitLab, reporting bugs and requesting features could continue to happen here on Wikipedia, as issues are intentionally disabled on gitlab.wikimedia.org. (I prefer gitlab.wikimedia.org to GitHub to keep the code in-house and make it easier to link Git users to Wikipedia users.) —Tacsipacsi (talk) 15:28, 8 December 2023 (UTC)
Refactor parsing of wikitext
This edit request has been answered. Set the |answered= or |ans= parameter to no to reactivate your request.
This is just the "Removed the custom wikitext parsing engine at wiki2html() and replaced it a call to the parsing API" part of your above code, and not the other two items, correct? Also, what is a good test procedure for this? –Novem Linguae (talk) 11:15, 6 December 2023 (UTC)
Oh, okay :) Since this is the preview module, anything that shows a preview would work (articles, userpages, images). I've put together a set of links at User:Sohom_Datta/sandy-box in case you want to test it out :) Sohom (talk) 11:55, 6 December 2023 (UTC)
Sandy box. Love that. Haha. In my testing, I found two differences between the old and the new.
Templates now render, instead of displaying the wikicode. You can see this in your sandy box image examples. I think this is a feature not a bug. I think folks will like this. Y
Headings now have [ edit | edit section ] links that they didn't have before. And they are showing up a bit too big. I think they are the same size as the headings rather than body text. I think this should either be shrunk or hidden before we deploy this patch. This may involve changing MediaWiki:Gadget-navpop.css. N
Now that the other patch was merged, needs a rebase. N
I won’t like if templates are rendered. Templates are not rendered for a reason: how templates making heavy use of parameters happen to render on their own page doesn’t say much about what they’re really are. They are oftentimes even partly or entirely <includeonly>d, so parts are left out from the preview. Couldn’t the namespace condition moved to before making the API request, so that the request is sent only for non-template pages, continuing to display template wikitext verbatim? —Tacsipacsi (talk) 15:37, 8 December 2023 (UTC)
I intend to take another stab at this next week, it seems to be fairly complicated to have templates not be rendered, I will see if we can maybe only the show the documentation somehow. Sohom (talk) 15:59, 15 December 2023 (UTC)
Is it possible to add the most recent block log entry (for currently blocked users) into the popup? Similar to how MediaWiki:Gadget-markblocked.js puts the reason in the tooltip. I have both scripts enabled, and it does strikethrough the username but Popups overrides the markblocked tooltip for blocked editors so I can't see the block reason without navigating away from the page I'm on to check the block log. I'd try building it myself based on the markblocked gadget, but my Javascript knowledge is very limited. The WordsmithTalk to me21:11, 28 February 2024 (UTC)
Want to sandbox it somewhere and make sure it works? Then tag this with {{IAER}}? Also, searching for "preview failed" reviews 5 spots we could apply this pattern to. –Novem Linguae (talk) 10:29, 31 January 2024 (UTC)
Implemented, thanks valepert. However, two things of note: first, there are a ton more error messages that are not similarly isolated. Second, I notice there's an unrelated bug in your example, where the % in the article title display is being unnecessarily URI-encoded, which breaks the link to the article itself. Writ Keeper⚇♔14:03, 29 February 2024 (UTC)
Done — xaosflux Talk 19:14, 28 October 2021 (UTC)
On hold leaving this open for a day to see if there are any other comments, otherwise seems fine and will do. — xaosflux Talk 15:07, 6 January 2022 (UTC)
Fixed. Thanks for the code review. I am disappointed in my IDE's linters for allowing me to type that and not putting a little red squiggle under it. –Novem Linguae (talk) 23:52, 15 August 2023 (UTC)
Not done: Please see TheDJ's comment. Izno (talk) 19:39, 3 June 2022 (UTC)
Not done this request is stale, and multiple other changes have been implemented that would have forked the version. Also, it doesn't appear to be uncontroversial. If you wish to proceed please continue the discussion and make a sandbox of the current version and your proposed diff again (you can just put it at Special:MyPage/Gadget-popups-sandbox.js for example. — xaosflux Talk 03:15, 8 October 2023 (UTC)