Description
Details
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| For interwiki links, set the nsText property to nil as well | mediawiki/extensions/Scribunto | master | +5 -0 |
Related Objects
Event Timeline
Change #1092899 had a related patch set uploaded (by Gerrit Patch Uploader; author: Anne Haunime):
[mediawiki/extensions/Scribunto@master] Add missing tests for nsText property of title objects
Fixed tests: gerrit 1092899/1..2.
That was precisely because of the issue detailed in T369784 (which is pending since July…). After the patch at hand is merged, gerrit 1053882 will need to be rebased (to update the tests added here)
Notice how nsText is, currently, diverging from subjectNsText and talkNsText.
We could (should) state that nsText is either subjectNsText or talkNsText… but currently there is the difference about the underscores.
Tests are still failing, and it's quite interesting.
Provided the object created by mw.title.new( 'interwikiprefix:Module:TestFramework' ):
- Its subjectNsText and talkNsText properties are nil.
- So naturally, I was expecting nsText to be nil as well. But currently, its value is '' (empty string)…
Related: https://gerrit.wikimedia.org/r/c/mediawiki/extensions/Scribunto/+/983410
I would suggest to be bold, and complete/correct gerrit 983410 by adding nsText to the list of properties that are set to nil. Considering how interwiki links are such a special case, I can't think of situations where this nsText change would break something.
I don't think that amendment is worth creating a dedicated Phabricator task (but correct me if I'm wrong).
Uploaded patchset 3. It now accomplishes two things:
- Sets nsText to nil for interwiki links, as pointed out above.
- Adds the missing unit tests, as originally intended.
Currently encountering an error during the unit tests, at this line: mw.title.lua#L91.
That is because of an attempt to concatenate the new value nil.
This can be fixed, however:
- We should set the interwiki property to nil as well, for consistency, and because we don't know the value of this property. (edit 2025-05-21: Actually, we do know the value of this property, don’t we?)
⚠️🤔 There is something I don't understand:- The current value of this property is '' (empty string): as obtained with mw.title.new( 'interwikiprefix:Module:TestFramework' ).interwiki
- However, the relevant unit test expects the value 'interwikiprefix': TitleLibraryTests.lua#L296.
- We should also set the namespace property to nil. Currently it contains 0, which is erroneous.
I would need to understand the situation regarding the interwiki property. As for the namespace property, we can postpone changing it to a later stage.
That's because MediaWiki checks whether the interwiki is a recognized one. If it's not recognized, it is basically handled as part of a local title.
See TitleParser.php#L235, and in particular TitleParser.php#L264:
} elseif ( $this->interwikiLookup->isValidInterwiki( $p ) ) {So, there should simply be a difference between WMF production wikis and the test environment; the former doesn't recognize it, whereas the latter does, likely due to some override that I haven't yet localized.
Anyway, setting additional properties to nil is not as straightforward as I expected, so I'm considering resetting the patch to its initial task: just adding the missing unit tests. Additional work later would be better suited to a new ticket.