Wikimedia Bugzilla is closed!

Note, said discussion hasn't concluded/been closed

it's closed now so you can deploy it. Thanks!

Thought I'd look over this extension and give some thoughts:


Line 28 of InteractiveBlockedMessage.php:
$wgAutoloadClasses['InteractiveBlockMessage'] = "$dir/InteractiveBlockMessage.php";

I'm not sure what that's about (It won't hurt anything, but there is no such class, so it probably shouldn't be there)

*LanguageGetMagic hook usage - That's probably ok, but if you use a .magic.i18n.php file, the magic word name can be translated at translatewiki (I think anyways)

*MagicWord id being all capitalized - Doesn't really matter, but the magic word id (not the actual magic word, just its internal id) are all lowercase for all the magic words in core

*$user->isBlocked() call - should perhaps pass a true to explicitly ask the result to come from slave (doesn't really matter though, since slave is default, but good to be explicit)

*Doesn't work on anon user pages (not sure if intentional or not. if it's wanted, all that needs to be done is pass false as second argument to user::newFromName) if it did work for anons, there might be security implications in revealing auto-blocks (As mentioned on the en wp page, although given the number of autoblocks in place at one time, it'd be difficult to glean much from this, but still not a good thing)

*If the user causes their own user page to be rendered, the magic word will report true if the user is blocked via auto-blocks, proxy auto-blocker, or DNSBL block (although I don't think the last two are used by wikimedia). This is inconsistent (Compared to other people rendering the page), and probably a bad thing (Although not horrible). Perhaps better to use Block::newFromTarget directly, in which case it would only look at explicit blocks (and could also be used on anon user pages without leaking auto-blocks).

*We should perhaps add a hook to SpecialUnblock::processUnblock so that this extension could explicitly purge user pages on an explicit unblock.

--
Cheers

-------
btw, does this extension still need review by the senior dev cabal? (I know there is a mailing list thread about that) if so, it should have the keyword "need-review" and be blocking bug 31235.

Baowolff, you missed Bug 32128 which this bug should depend on.  I've pointed to your review there.

(In reply to comment #4)
> Baowolff, you missed Bug 32128 which this bug should depend on.  I've pointed
> to your review there.

bleh.  Forget this. I screwed up.

Petr, please respond to these criticisms from bawolff.  Thanks.

English Wikipedia templates are copied to other wikis very often. I'm not sure adding an extension dependency is a good idea here. Is there a reason an extension was chosen rather than implementing this magic word in core?

-shell as it's not ready for deploy

Petr, just pinging to see whether you have had time to look at the code critique. Thanks!

Petr: Could you comment on comment 3 please?


https://www.mediawiki.org/wiki/Extension:InteractiveBlockMessage links to SVN (not converted to using Git yet so likely not actively developed?).

(In reply to comment #10)
> Petr: Could you comment on comment 3 please?

Comment 7 as well. :-)

I think this small (~30 lines) obscure templating feature should just be in the core.

(In reply to comment #10)
> Petr: Could you comment on comment 3 please?
> 
> 
> https://www.mediawiki.org/wiki/Extension:InteractiveBlockMessage links to SVN
> (not converted to using Git yet so likely not actively developed?).

Looks like it is in Git: https://gerrit.wikimedia.org/r/gitweb?p=mediawiki/extensions/InteractiveBlockMessage.git;a=summary

(In reply to comment #12)
> I think this small (~30 lines) obscure templating feature should just be in
> the
> core.

I would prefer to have core as small as possible and as fast as possible. But given the amount of mess that is already in core, I think that inserting some 30 lines more wouldn't hurt. So... I don't have problem with that.

Updating the bug summary to reflect the shift from deploying a particular MediaWiki extension (InteractiveBlockMessage) to Wikimedia wikis to putting this feature/functionality in MediaWiki core.

Marking this bug as "easy" as most of the code is already written, I believe, it just needs to be put into MediaWiki core in a Gerrit changeset.

(In reply to comment #3)
> Thought I'd look over this extension and give some thoughts:
> 
> 
> Line 28 of InteractiveBlockedMessage.php:
> $wgAutoloadClasses['InteractiveBlockMessage'] =
> "$dir/InteractiveBlockMessage.php";
> 
> I'm not sure what that's about (It won't hurt anything, but there is no such
> class, so it probably shouldn't be there)
> 

if it's not there, be bold and fix it. I am sure it was there in past.

> *LanguageGetMagic hook usage - That's probably ok, but if you use a
> .magic.i18n.php file, the magic word name can be translated at translatewiki
> (I
> think anyways)
> 

translating magic words? Nooooo please :) keep it compatible cross-language (magic words are already translated in Microsoft Excel and it's pain in the arse)

> *MagicWord id being all capitalized - Doesn't really matter, but the magic
> word
> id (not the actual magic word, just its internal id) are all lowercase for
> all
> the magic words in core
> 
> *$user->isBlocked() call - should perhaps pass a true to explicitly ask the
> result to come from slave (doesn't really matter though, since slave is
> default, but good to be explicit)
> 

so fix it.

> *Doesn't work on anon user pages (not sure if intentional or not. if it's
> wanted, all that needs to be done is pass false as second argument to
> user::newFromName) if it did work for anons, there might be security
> implications in revealing auto-blocks (As mentioned on the en wp page,
> although
> given the number of autoblocks in place at one time, it'd be difficult to
> glean
> much from this, but still not a good thing)
> 

well, the original purpose was to make block messages on anon pages less confusing. But given the incredible amount of bureaucracy on english wikipedia, who knows if it's even possible since the question "am I even allowed to edit wikipedia" is TOP SECRET now.

> *If the user causes their own user page to be rendered, the magic word will
> report true if the user is blocked via auto-blocks, proxy auto-blocker, or
> DNSBL block (although I don't think the last two are used by wikimedia). This
> is inconsistent (Compared to other people rendering the page), and probably a
> bad thing (Although not horrible). Perhaps better to use Block::newFromTarget
> directly, in which case it would only look at explicit blocks (and could also
> be used on anon user pages without leaking auto-blocks).
> 
> *We should perhaps add a hook to SpecialUnblock::processUnblock so that this
> extension could explicitly purge user pages on an explicit unblock.
> 

let's fix it

> --
> Cheers
> 
> -------
> btw, does this extension still need review by the senior dev cabal? (I know
> there is a mailing list thread about that) if so, it should have the keyword
> "need-review" and be blocking bug 31235.

This extension should not be deployed. The feature is reasonable for MediaWiki core, though.

Removing the blocks bug 31235 as this is no longer about deploying an extension. Feel free to reblock that tracking bug if it switches back.

The blocks were removed and yet this has been sitting here for months... Adding myself to cc list and making a comment to bump this and move it forward (perhaps).

I think bug 25380 is a similar, related request, except that magic word can take a parameter after the colon. Is there any other difference between these two?

Change 133661 had a related patch set uploaded by Withoutaname:
Add new ParserFunction "USERBLOCKED"

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