Jump to content

MediaWiki talk:Gadget-popups.js

Page contents not supported in other languages.
From Wikipedia, the free encyclopedia

Certain user permissions (extended confirmed, global renamer) not showing

[edit]

Usually when you hover over a username, popups shows all their local permissions in regular text, and all their global permissions in italics. However, I've noticed a couple permissions are not showing up. In particular, extended confirmed (local) and global renamer (global). I propose fixing these two, and doing an audit to make sure there's not any other permissions being missed. If there's some hard-coded list in the code somewhere, I propose switching to something that is not hard-coded so that new permissions can be reliably detected without code modifcations. –Novem Linguae (talk) 04:13, 16 April 2024 (UTC)[reply]

I believe extendedconfirmed is an intentional omission. (No opinion in whether it should be.) Global renamer is, technically speaking, a local right on Meta, not a global right. See e.g. Special:CentralAuth/Tamzin. As a global renamer, I'd definitely find it useful to have that included here, but I'd think it'd require an extra API call just for that check? -- Tamzin[cetacean needed] (they|xe) 04:21, 16 April 2024 (UTC)[reply]
Yeah, looks like global renamer is local to meta and would require its own API call. Good find, thank you. –Novem Linguae (talk) 04:36, 16 April 2024 (UTC)[reply]
There could be a case for a no-permissions global group for global renamers, similar to VRT permissions agents, but I guess that would be a matter for Meta. -- Tamzin[cetacean needed] (they|xe) 04:38, 16 April 2024 (UTC)[reply]
"I believe extendedconfirmed is an intentional omission." correct. there are several groups (not permissions btw) that are filtered out because almost everyone has them. —TheDJ (talkcontribs) 08:22, 16 April 2024 (UTC)[reply]

eslint-config-wikimedia autofixes

[edit]

In a sandbox, I ran the official wikimedia javascript linter on the popups code. Here's the diff.

  • ran eslint-config-wikimedia autofixes
    • extends { "wikimedia/client/es6", "wikimedia/jquery", "wikimedia/mediawiki" }
    • excluding
      • controversial ones such as space-in-parens and max-len
      • no-var (changing var to let/const). this would be fine to do since Grade A support is on ES6 now, but will save for a future patch
      • unicorn/prefer-string-slice. I'm not sure the suggested .slice( 0, Math.max( is more readable/better than the existing .substring(
      • some rules that weren't auto-fixing well when I manually reviewed the code
    • including rules such as comma-dangle, no-unneeded-ternary, no-useless-concat, operator-linebreak, semi-style, unicorn/prefer-date-now, dot-notation, jsdoc/check-tag-names, jsdoc/check-types, and others
  • removed some comments

Will self-merge in a couple days if no objections.

Note to self: Manually test a bit on testwiki before deploying. –Novem Linguae (talk) 06:36, 25 April 2024 (UTC)[reply]

I really dislike the "comma-dangle": "never" setting, it makes subsequent diffs less readable (and accounts for a large part of the current diff as well). Could you change it to only-multiline (or always-multiline if you prefer) and revert your changes that removed trailing commas? Thanks in advance!
Also, I don’t see any reference to this ESLint rule set in the code. Could you add a comment (even if not machine-readable) that informs future maintainers what they should lint the code with? —Tacsipacsi (talk) 19:41, 26 April 2024 (UTC)[reply]
Thanks for the feedback. I'll avoid comma-dangle for now. Although it's in the official Wikimedia JavaScript linter, so I'm sure they have their reasons.
It'd be a pretty big comment, so I will probably have to keep it out of the code for now. The file I am using is...
{
	"extends": [
		"wikimedia/client/es6",
		"wikimedia/jquery",
		"wikimedia/mediawiki"
	],
	"globals": {
		"mw": "readonly"
	},
	"parserOptions": {
		"ecmaVersion": "latest",
		"sourceType": "module"
	},
	"rules": {
		"camelcase": "off",
		"es-x/no-array-prototype-includes": "off",
		"es-x/no-async-functions": "off",
		"es-x/no-class-fields": "off",
		"es-x/no-optional-catch-binding": "off",
		"es-x/no-regexp-lookbehind-assertions": "off",
		"es-x/no-regexp-s-flag": "off",
		"es-x/no-rest-spread-properties": "off",
		"es-x/no-string-prototype-matchall": "off",
		"es-x/no-string-prototype-replaceall": "off",
		"jsdoc/require-param": "off",
		"jsdoc/require-param-type": "off",
		"jsdoc/require-returns": "off",
		"max-len": "off",
		"mediawiki/class-doc": "off",
		"no-jquery/no-class-state": "off",
		"no-jquery/no-global-selector": "off",
		"no-jquery/no-parse-html-literal": "off",
		"no-jquery/variable-pattern": "off",
		"no-shadow": "off",
		"no-underscore-dangle": "off",
		"security/detect-non-literal-regexp": "off",
		"security/detect-unsafe-regex": "off",
		"unicorn/prefer-string-slice": "off",
		"space-in-parens": "off",
		"computed-property-spacing": "off",
		"eqeqeq": "off",
		"no-var": "off",
		"spaced-comment": "off",
		"array-bracket-spacing": "off",
		"jsdoc/no-multi-asterisks": "off",
		"no-alert": "off",
		"no-tabs": "off",
		"jsdoc/require-asterisk-prefix": "off",
		"jsdoc/check-alignment": "off"
	}
}
Novem Linguae (talk) 13:14, 4 May 2024 (UTC)[reply]
 Done. –Novem Linguae (talk) 15:14, 4 May 2024 (UTC)[reply]
@Novem Linguae: Thanks for excluding comma-dangle!

Although it's in the official Wikimedia JavaScript linter, so I'm sure they have their reasons.

I guess simply legacy. According to ECMAScript version history#5th Edition – ECMAScript 2009, a trailing comma in object literals is accepted only since ES5, and Wikipedia used to support ES3.

It'd be a pretty big comment, so I will probably have to keep it out of the code for now.

I see. Then maybe a subpage or a collapsible box on the top of this talk page? I fear a random talk page topic will get lost in the archives.
(Sorry for the late reply, I missed notifications due to a bot archival, which stopped watchlist emails despite not sending one itself.) —Tacsipacsi (talk) 10:32, 18 May 2024 (UTC)[reply]
I was reading a random Phab ticket today and found out why comma-dangle is in eslint-config-wikimedia. It's for easier copy-pasting to JSON. I guess this use case was important to Wikimedia developers back when they decided on this rule. Today I learned. –Novem Linguae (talk) 15:53, 1 June 2024 (UTC)[reply]
What is that random Phab task? I find this hard to believe (or at least not well thought through) because at the same time we have "quotes": "single", which is exactly the non-JSON-compatible version, and it’s even much more work to replace all the single quotes with double ones than deleting some commas. —Tacsipacsi (talk) 21:47, 1 June 2024 (UTC)[reply]
phab:T222707#5164890 and phab:T222707#5165095
(the developer community has had this conversation before and decided that easier portability to JSON is more important than cleaner diffs)Novem Linguae (talk) 00:08, 2 June 2024 (UTC)[reply]

Updates for dark mode

[edit]

Can an interface admin please copy User:The wub/popups sandbox.css to MediaWiki:Gadget-navpop.css (comparison). This updates the styling to work in the new dark mode (phab:T365749), following the guidance at mw:Recommendations for night mode compatibility on Wikimedia wikis. the wub "?!" 20:32, 26 May 2024 (UTC)[reply]

 Done * Pppery * it has begun... 01:55, 27 May 2024 (UTC)[reply]
Thanks very much! the wub "?!" 09:07, 27 May 2024 (UTC)[reply]
@Pppery: Another edit, sorry. It was reported that forcing the diff text color clashes with the "Blackskin" gadget. I figured out some new diff background colors for dark mode which work with the white text color, avoiding the issue with Blackskin and I think looking better on the whole. Can you update from my sandbox again please? Here's the diff. the wub "?!" 21:43, 27 May 2024 (UTC)[reply]
 Done * Pppery * it has begun... 01:31, 28 May 2024 (UTC)[reply]

I became aware of an issue with Navigation Popups through a discussion started by @Tacsipacsi in translatewiki.net, which is the same issue @MarcoAurelio reported in phab:T329217.

I believe the fix for this is quite simple, and can be achieved by changing this:

	function setMainRegex() {
		var reStart = '[^:]*://';
		var preTitles =
			literalizeRegex(mw.config.get('wgScriptPath')) + '/(?:index[.]php|wiki[.]phtml)[?]title=';
		preTitles += '|' + literalizeRegex(pg.wiki.articlePath + '/');

		var reEnd = '(' + preTitles + ')([^&?#]*)[^#]*(?:#(.+))?';
		pg.re.main = RegExp(reStart + literalizeRegex(pg.wiki.sitebase) + reEnd);
	}

into this:

	function setMainRegex() {
		var reStart = '[^:]*://';
		var preTitles =
			'(?:' + literalizeRegex(mw.config.get('wgScript')) + '|' +
			literalizeRegex(mw.config.get('wgScriptPath')) + '/(?:index[.]php|wiki[.]phtml))[?]title=';
		preTitles += '|' + literalizeRegex(pg.wiki.articlePath + '/');

		var reEnd = '(' + preTitles + ')([^&?#]*)[^#]*(?:#(.+))?';
		pg.re.main = RegExp(reStart + literalizeRegex(pg.wiki.sitebase) + reEnd);
	}

But someone who's familiar with this gadget should test it properly before adding the fix. I've tested it locally in the browser console both here and on translatewiki.net, and it works well in both places, but I'm not very familiar with this script, so someone who is should test this before making the changes. Jon Harald Søby (talk) 20:01, 22 December 2024 (UTC)[reply]

Comments would be helpful. So maybe...
function setMainRegex() {
	var reStart = '[^:]*://';
	var preTitles =
		// translatewiki.net uses i.php instead of index.php
		'(?:' + literalizeRegex(mw.config.get('wgScript')) + '|' +
		// handle the more common cases of index.php and wiki.phtml
		literalizeRegex(mw.config.get('wgScriptPath')) + '/(?:index[.]php|wiki[.]phtml))';
	// Hmm. Why is there a | in this one? It doesn't appear to be inside of a (?:
	preTitles += '[?]title=|' + literalizeRegex(pg.wiki.articlePath + '/');
	var reEnd = '(' + preTitles + ')([^&?#]*)[^#]*(?:#(.+))?';
	pg.re.main = RegExp(reStart + literalizeRegex(pg.wiki.sitebase) + reEnd);
}
I am confused by one of the lines, so I added a comment asking about it. Feel free to continue refactoring and fixing the comments. –Novem Linguae (talk) 20:42, 22 December 2024 (UTC)[reply]
@Novem Linguae: Sure, comments are fine of course. 😊 (But if I were to nitpick: I'm not too sure about calling wiki.phtml a "common case", haha.)
The line with the | is put into parentheses in the var reEnd line. So the part before the bar is /w/index.php type stuff, while the part after it is /wiki/ type stuff, and then the part that comes after them both in reEnd is the page title. Jon Harald Søby (talk) 22:12, 22 December 2024 (UTC)[reply]
What if index.php was dropped from the “more common cases” branch? On wikis without exotic configuration (i.e. all WMF wikis, for example), this would make no difference other than making the regex simpler. On wikis like Translatewiki, this would break NavPopups for about ten index.php links (lot of the results are false positives like user scripts and stack traces). And there was no problem with the naming: the remaining wiki.phtml is definitely “legacy” rather than “common”. —Tacsipacsi (talk) 22:34, 22 December 2024 (UTC)[reply]
Maybe something like
	function setMainRegex() {
		var reStart = '[^:]*://';
		var preTitles =
			literalizeRegex(mw.config.get('wgScript')) + '[?]title=';
		preTitles += '|' + literalizeRegex(pg.wiki.articlePath + '/');
		var reEnd = '(' + preTitles + ')([^&?#]*)[^#]*(?:#(.+))?';
		pg.re.main = RegExp(reStart + literalizeRegex(pg.wiki.sitebase) + reEnd);
	}
might be more cleaner? This should cater to the usecase of translatewiki.net while also geting rid of the legacy code and hardcoded values? Sohom (talk) 23:29, 22 December 2024 (UTC)[reply]
The problem with this is that the “legacy code” is there to recognize the legacy links that exist in content; those links aren’t automagically fixed. There are over 18,000 results for wiki.phtml on enwiki alone. (wgScript specifies the what URLs MediaWiki generates, while this regex tries to find the URLs MediaWiki is reachable at. The latter is of course a proper superset of the former, in order to make gadget authors’ lives more challenging. ) —Tacsipacsi (talk) 19:49, 23 December 2024 (UTC)[reply]
@Tacsipacsi, @Sohom Datta: I don't think it's a good idea to drop the "hard-coded" index.php; imagine the scenario where a wiki for some reason decides to change their wgScript to something else after existing for a few years. All old links that people have just copy-pasted will then still include index.php and not the new config setting (just as is the case with wiki.phtml here in enwiki). So I think it makes sense to include a hard-coded index.php in addition to the wgScript, even though they will be the same in most cases. Jon Harald Søby (talk) 15:53, 25 December 2024 (UTC)[reply]

@Novem Linguae: Okay, so final (hopefully?) suggestion based on the feedback (I've changed the comments slightly to generalize them):

function setMainRegex() {
	var reStart = '[^:]*://';
	var preTitles =
    	// Take customizable wgScript into account (it isn't guaranteed to be index.php)
		'(?:' + literalizeRegex(mw.config.get('wgScript')) + '|' +
		// handle index.php (likely to work even if different from wgScript) and legacy wiki.phtml
		literalizeRegex(mw.config.get('wgScriptPath')) + '/(?:index[.]php|wiki[.]phtml))';
	preTitles += '[?]title=|' + literalizeRegex(pg.wiki.articlePath + '/');
	var reEnd = '(' + preTitles + ')([^&?#]*)[^#]*(?:#(.+))?';
	pg.re.main = RegExp(reStart + literalizeRegex(pg.wiki.sitebase) + reEnd);
}

Is this OK? Jon Harald Søby (talk) 15:53, 25 December 2024 (UTC)[reply]

CR+1 –Novem Linguae (talk) 18:21, 25 December 2024 (UTC)[reply]
CR+2 and  Done. Sohom (talk) 14:48, 26 December 2024 (UTC)[reply]
Thank you! :-) Jon Harald Søby (talk) 14:57, 27 December 2024 (UTC)[reply]

str.includes is not a function

[edit]

When hovering over files in a category, like c:Category:Down_East_Latch_Strings_(1887), I get

Uncaught TypeError: str.includes is not a function
    isValidImageName https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-popups.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400:3222
    loadImage https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-popups.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400:5547
    nonsimplePopupContent https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-popups.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400:450
    mouseOverWikiLink2 https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-popups.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400:354
    mouseOverWikiLink https://en.wikipedia.org/w/index.php?title=MediaWiki:Gadget-popups.js&action=raw&ctype=text/javascript&smaxage=21600&maxage=86400:232
index.php:3222:15

This appears to be caused by Special:Diff/1265443931 (@Novem Linguae). AntiCompositeNumber (talk) 00:12, 27 December 2024 (UTC)[reply]

Dang. Usually eslint autofixes don't cause bugs. I'll have to revert and be more cautious. Will revert in a moment. –Novem Linguae (talk) 00:20, 27 December 2024 (UTC)[reply]
function isValidImageName(str) {
	// extend as needed...
	return str.indexOf('{') == -1;
}
On the page you linked above, hovering over a random file as you stated, my breakpoint in that code says that "str" is a Title object with the keys of "anchor" and "value". Seems a bit GIGO to me. Whenever a Title object is passed instead of a string, I think it is always going to return true instead of actually checking for "{".
Bit of a rabbit hole though. I probably won't mess with it further. Will just skip the unicorn/prefer-includes autofix the next time I work on this. –Novem Linguae (talk) 00:36, 27 December 2024 (UTC)[reply]

Bug: Popup uses HTML em and strong in place of i and b

[edit]

Example: monkfruit

  • From page:
    <i><b>Siraitia grosvenorii</b></i>, also known as <b>monkfruit</b>, <i><b>luo han guo</b></i>, or <b>Swingle fruit</b> ...
  • From popup:
    <strong><em>Siraitia grosvenorii</em></strong>, also known as <strong>monkfruit</strong>, <strong><em>luo han guo</em></strong>, or <strong>Swingle fruit</strong> ...

The problem is that <i> and <b> are formatting tags while <em> and <strong> are semantic tags. They're not interchangeable even though the result looks the same in most cases. Myself, I have a CSS rule for <em> so that I can tell it apart from regular italics.

I'm not great with JS, but I only see <em> and <strong> in the source code in one spot, so it looks like you'd just need to change this:

o += st ? '<strong>' : '</strong>';
+
o += st ? '<b>' : '</b>';
o += em ? '<em>' : '</em>';
+
o += em ? '<i>' : '</i>';

W.andrea (talk) 19:44, 22 February 2025 (UTC)[reply]

There was a time that W3C attempted to deprecate the b and i elements, suggesting that we use CSS instead. So people switched to the strong and em elements since they're much easier, and also look the same (and who cared about semantics back then?). It's not exactly a bug, though, is it? Are we getting complaints from WP:WCAG? --Redrose64 🌹 (talk) 21:58, 22 February 2025 (UTC)[reply]
It is a bug per MOS:EMPHASIS and MOS:BOLD, which say em and i, and strong and b are distinct:

The most accessible way to indicate emphasis is with the HTML <em>...</em> element or by enclosing the emphasized text within an {{em|...}} template. Italics markup (''...'', or <i>...</i>) is often used in practice for emphasis, but this use is not semantically correct markup, so emphasis markup is preferred. Italics markup is for non-emphasis purposes, such as for book titles and non-English language phrases, as detailed below.

To create [boldface], surround the text ... with triple apostrophes.
To denote importance, seriousness, or urgency using semantic markup, you can also use the HTML element <strong>...</strong>, or the template {{strong}} ...

W.andrea (talk) 02:58, 23 February 2025 (UTC) edited 03:26, 23 February[reply]
A bug? As the output of popups is not part of any rendered Wikipedia page, it seems unnecessary nit picking to spend efforts on this. -- Michael Bednarek (talk) 02:12, 23 February 2025 (UTC)[reply]
I wouldn't describe a disregard of W3C guidelines in a barely visible popup a bug. -- Michael Bednarek (talk) 03:44, 23 February 2025 (UTC)[reply]
How would you describe it then? — W.andrea (talk) 12:40, 23 February 2025 (UTC)[reply]
What do you mean by "barely visible"? — W.andrea (talk) 12:44, 23 February 2025 (UTC)[reply]
I would describe it as an infelicity. It's barely visible because, a) it's not part of a rendered Wikipedia page, so no W3C syntax checking tool will ever see it; b) readers who have activated this gadget are a minuscule minority; c) and even they won't face its output when they use a mobile device. IOW, much ado about nothing. -- Michael Bednarek (talk) 15:28, 23 February 2025 (UTC)[reply]
Could you define "infelicity"? I checked Webster which refers to "infelicitous" which says "not felicitous", which says "apt; pleasant, delightful", none of which seem to fit this context. I guess you could say "imperfection"? — W.andrea (talk) 15:50, 23 February 2025 (UTC)[reply]
It seems like we can't agree where this comment goes, since all of us have moved it at some point :) @Michael Bednarek, you initially posted this comment in reply to @Redrose64's comment, but it doesn't seem to make sense in that context; it seems like you were replying to my initial comment instead. Could you clarify? My reply was intended for Redrose64. Sorry if I stepped on anyone's toes here. — W.andrea (talk) 15:44, 23 February 2025 (UTC)[reply]
Likely the reason Popups outputs what it does is: it's an ancient gadget from when MediaWiki temporarily did the same with the relevant wikitext markup (IIRC that was a thing), or it's an ancient gadget from when the W3C said you shouldn't use i/b and the developers of the gadget then said "ok we'll use whatever we want". I would guess the relevant change would be at function parse_inline_formatting(str) and appears trivial - and from that perspective I don't understand the moaning just above about "you should do other things with your time".
I think lining up this bit with what the actual parsers plural output would be preferable even if it weren't a reasonable question of W3C standards conformity (and I agree that em and strong are categorically wrong).
Novem Linguae was last mucking around and I know they've considered off-site development before, so I guess I'll ping them to see if they're maintaining another version of this elsewhere or if this is the authoritative copy. If not, I can make this change. Izno (talk) 19:00, 23 February 2025 (UTC)[reply]
The on site copy is the authoritative one. Thanks for checking. Feel free to implement if you think it has consensus. –Novem Linguae (talk) 23:51, 23 February 2025 (UTC)[reply]
I'm not opposed to implementing this, however, I find this a bit pointless and the argument for implementing it are weak, considering the state of the code (which is a mess). Sohom (talk) 23:57, 23 February 2025 (UTC)[reply]
 Done Izno (talk) 02:46, 24 February 2025 (UTC)[reply]
Better is better, itches get scratched, and we move on with someone(s) happier and nobody worse-off. A pile of ancient and messy code that is still in production-use and becomes 1% better and has new eyes looking at it? That's clearly a win. DMacks (talk) 03:05, 24 February 2025 (UTC)[reply]