Page MenuHomePhabricator

Enable wgRelevantUserName for IP ranges
Open, Needs TriagePublic

Description

wgRelevantUserName was added to MW in 1.23, but it doesn't exist when viewing an IP range on (at least) Special:Block and Special:Contributions (compare 192.168.0.2 and 192.168.0.0/16). Some awkward workarounds are possible but inconsistent (using #mw-bi-target or input[name=wpTarget] on Special:Block, input[name=target].mw-input on Special:Contributions); it'd be nice to have a unified and reliable way to get it, regardless of user type.

Of note: it seems to always exist on Special:DeletedContributions, and does so on Special:Log iff the range is given with the user= parameter (e.g., https://en.wikipedia.org/wiki/Special:Log?user=192.168.0.0%2F16).

Event Timeline

It turns out to be intentional - see highlighted lines of SpecialContributions.php. However, I cannot figure out why the comments say a 'logs' link (of an IP range) may be irrelevant and am looking forward to a reply from those who know it.

@Amorymeltzer Oh yeah I got it. IMHO it would be more appropriate to enable it while do some additional check in skin stuff like SkinTemplate.php.

@Anomie I actually meant to update this to something like your comment in T239527#5709615 after playing around with things more recently; there's enough of a difference in how we'd want to treat what could be called a "user" (account, single IP) and what could have edits displayed (that + range) that I think that's the right course.

Commenting as an admin + Twinkle user (pretty sure @Amorymeltzer has TW in mind here) that this functionality would be very useful - I routinely use Twinkle and I make rangeblocks fairly often, but when making rangeblocks my workflow is disrupted since Twinkle can't block from Special:Contribs/(IP)/(mask) so I have to use the normal block interface.

Change 642610 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Define Skin RelevantUser for IP ranges

https://gerrit.wikimedia.org/r/642610

Jdlrobson added a subscriber: Jdlrobson.

I think this would benefit from some input from platform engineering.

Change 642610 merged by jenkins-bot:
[mediawiki/core@master] Define Skin RelevantUser for IP ranges

https://gerrit.wikimedia.org/r/642610

Jdlrobson claimed this task.
Ammarpad removed Jdlrobson as the assignee of this task.
Ammarpad added a subscriber: Ammarpad.

This is not resolved unfortunately... something was missed

This is not resolved unfortunately... something was missed

Why do you say that? It works on the beta cluster for me, tested at https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Contributions/1.1.1.1/30, and won't reach production until January

Change 650026 had a related patch set uploaded (by Ammarpad; owner: Ammarpad):
[mediawiki/core@master] Fix Skin::getRelevantUser to handle IP ranges

https://gerrit.wikimedia.org/r/650026

This is not resolved unfortunately... something was missed

Why do you say that? It works on the beta cluster for me, tested at https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Contributions/1.1.1.1/30, and won't reach production until January

That's because Special:Contributions sets the relevant user itself. On places where the Skin will extract it, it won't work. And in fact the links will not only be created, but they'll point to wrong user: https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:1.1.1.1/30.

Not sure I understand what's broken here.
I can see mw.config.get('wgRelevantUserName') is correct and on https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:1.1.1.1/30 I see a link in the sidebar to https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Contributions/1.1.1.1

On places where the Skin will extract it, it won't work.

Can you give an example of a page where it's not working?
Should we revert the last change? I'd rather lot leave this in a broken state and try again in your new patch if you are saying you missed something in the implementation. Let's also add tests next time round.

Not sure I understand what's broken here.
I can see mw.config.get('wgRelevantUserName') is correct and on https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:1.1.1.1/30

What did you see? The relevant username is "1.1.1.1/30 ", if you look well, you'll see it's not what's in the console.

I see a link in the sidebar to https://meta.wikimedia.beta.wmflabs.org/wiki/Special:Contributions/1.1.1.1

That's because that link is to contribution page of a single "IP" not "IP range". If you try this, https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:1.1.1.1/30 (which is for range talk, you'll see the links too)

But, they are not supposed to be there according to the code. That's what the follow-up patch fixed.

// If there's a relevant user and is not IP range, set sidebar
// 'Tools' links.  IP ranges excluded because some of the links
// don't make sense to them.
$user = $this->getRelevantUser();
if ( $user && !IPUtils::isValidRange( $user ) ) {
    ...
}

On places where the Skin will extract it, it won't work.

Can you give an example of a page where it's not working?

I have already given an example above. https://meta.wikimedia.beta.wmflabs.org/w/index.php?title=User:1.1.1.1/30

Should we revert the last change? I'd rather lot leave this in a broken state and try again in your new patch if you are saying you missed something in the implementation. Let's also add tests next time round.

I have already fixed it https://gerrit.wikimedia.org/r/c/650026. Tests has been added https://gerrit.wikimedia.org/r/c/650032

Will this/these define wgRelevantUsername for a range outside of the CIDR limit? Similar to your proposed work in T211910 @Ammarpad, but curious to know.

Will this/these define wgRelevantUsername for a range outside of the CIDR limit? Similar to your proposed work in T211910 @Ammarpad, but curious to know.

I think this is currently not taken into account. The relevant user is set before CIDR limit check. But should we care?

I think the fact is we do have 'relevantUser' even if they're not blockable or some actions don't apply to them. Their status does not affect the relevant user-ness fact. In some wikis, maybe you can even find real users in 'unblockables' group, I guess there's an extension for that.

I suppose it depends on how you view wgRelevantUserName. It doesn't exist for non-existent usernames, which maybe implies it shouldn't be defined outside CIDR limits? Regardless, that fact makes wgRelevantUserName a reliable "does this single editor exist?" marker. IPs and specifically IP ranges are clearly a different beast, but at the moment, wgRelevantUserName indicates a working single user (ip or registered username), and is unhelpful for ranges. After this, it will indicate a registered user, a single ip, or a range; if it doesn't take into account CIDR limits, then maybe it's sort of a "anything I see would have a talk page" marker, but it would no longer be a reliable marker for "can I do things (e.g. block) with this?" On the flip side, something like mw.util.isIPv4Address doesn't bother with CIDR limits, so one might not expect this to, either.

This comment was removed by Amorymeltzer.

Change 650574 had a related patch set uploaded (by Jdlrobson; owner: Ammarpad):
[mediawiki/core@master] Define Skin RelevantUser for IP ranges

https://gerrit.wikimedia.org/r/650574

Change 650574 abandoned by Ammarpad:

[mediawiki/core@master] Define Skin RelevantUser for IP ranges

Reason:

https://gerrit.wikimedia.org/r/650574

Change 650026 abandoned by Ammarpad:

[mediawiki/core@master] Fix Skin::getRelevantUser to handle IP ranges

Reason:

https://gerrit.wikimedia.org/r/650026