Page MenuHomePhabricator

Field "hideuser" on Special:Block should be hidden when time field not indefinite
Closed, ResolvedPublic

Description

Users with the hideuser right have an extra checkmark on special:block to hide user. This only applies for indef blocks. If the duration drop down is set to something other then indef or custom, the hideuser field should be hidden with javascript, similar to what mediawiki.special.block.js does for other fields

Event Timeline

Bawolff created this task.Apr 19 2016, 12:50 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptApr 19 2016, 12:50 PM
Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptMay 1 2016, 8:06 PM

@Margott: This issue has been assigned to you a while ago.
Could you please share a status update? Are you still working (or still plan to work) on this issue? Is there anything that others could help with?
Only in case you do not plan to work on this issue anymore, should you be removed as assignee (via 'Assign / Claim' in the 'Actions' dropdown menu)?

Oh sorry i thought we finished this task. What else should i improve? So the whole logic is wrong?

Oh sorry i thought we finished this task.

@Margott: I see; thanks for explaining! :) Is there some related code change that you could link to? Thanks!

@Aklapper Yes i did, last time we did the git uploader and this time Matt suggested git review for these other changes. And im having some difficulties with the git review installation. I will recheck it again and if i cannot make it to upload via git review we have to find another option. So sorry about this delayer but this is my first time with git and phabircator.

@Aklapper Yes i did, last time we did the git uploader and this time Matt suggested git review for these other changes.

Ah great! Could you link to your change on Gerrit, please?

@Aklapper Yes i did, last time we did the git uploader and this time Matt suggested git review for these other changes.

Ah great! Could you link to your change on Gerrit, please?

Thats what im trying to, but for some reasons i cannot install gerrit and upload changes. Lets do it like this: check if the code is ok, and if its not suggest me what to change till i do i right, and then i spend more time on installation and upload it right on the next tasks.

/*!

  • JavaScript for Special:Block */

( function ( mw, $ ) {
$( function () {

		var $blockTarget = $( '#mw-bi-target' ),
			$anonOnlyRow = $( '#mw-input-wpHardBlock' ).closest( 'tr' ),
			$enableAutoblockRow = $( '#mw-input-wpAutoBlock' ).closest( 'tr' ),
			$hideUser = $( '#mw-input-wpHideUser' ).closest( 'tr' ),
			$watchUser = $( '#mw-input-wpWatch' ).closest( 'tr' ),
			$expiry = $( '#mw-input-wpExpiry' );

		function updateBlockOptions( instant ) {
			var blocktarget = $.trim( $blockTarget.val() ),
				isEmpty = blocktarget === '',
				isIp = mw.util.isIPAddress( blocktarget, true ),
				isIpRange = isIp && blocktarget.match( /\/\d+$/ ),
				expiryValue = $expiry.val(),
				isIndefinite = expiryValue === 'infinite',
				isNonEmptyIp = isIp && !isEmpty;

			if ( isNonEmptyIp ) {
				$enableAutoblockRow.goOut( instant );
			} else {
				$enableAutoblockRow.goIn( instant );
			}
			if ( isNonEmptyIp || ( isIndefinite ) ) {
				$hideUser.goOut( instant );
			} else {
				$hideUser.goIn( instant );
			}
			if ( !isIp && !isEmpty ) {
				$anonOnlyRow.goOut( instant );
			} else {
				$anonOnlyRow.goIn( instant );
			}
			if ( isIpRange && !isEmpty ) {
				$watchUser.goOut( instant );
			} else {
				$watchUser.goIn( instant );
			}
		}

		if ( $blockTarget.length ) {
			// Bind functions so they're checked whenever stuff changes
			$blockTarget.keyup( updateBlockOptions );
			$expiry.change( updateBlockOptions );

			// Call them now to set initial state (ie. Special:Block/Foobar?wpBlockExpiry=2+hours)
			updateBlockOptions( /* instant= */ true );
		}

} );
}( mediaWiki, jQuery ) );

Putting the previous comment into a block of three backticks so it is rendered properly:

/*!
 * JavaScript for Special:Block
 */
( function ( mw, $ ) {
	$( function () {
		var $blockTarget = $( '#mw-bi-target' ),
			$anonOnlyRow = $( '#mw-input-wpHardBlock' ).closest( 'tr' ),
			$enableAutoblockRow = $( '#mw-input-wpAutoBlock' ).closest( 'tr' ),
			$hideUser = $( '#mw-input-wpHideUser' ).closest( 'tr' ),
			$watchUser = $( '#mw-input-wpWatch' ).closest( 'tr' ),
			$expiry = $( '#mw-input-wpExpiry' );

		function updateBlockOptions( instant ) {
			var blocktarget = $.trim( $blockTarget.val() ),
				isEmpty = blocktarget === '',
				isIp = mw.util.isIPAddress( blocktarget, true ),
				isIpRange = isIp && blocktarget.match( /\/\d+$/ ),
				expiryValue = $expiry.val(),
				isIndefinite = expiryValue === 'infinite',
				isNonEmptyIp = isIp && !isEmpty;

			if ( isNonEmptyIp ) {
				$enableAutoblockRow.goOut( instant );
			} else {
				$enableAutoblockRow.goIn( instant );
			}
			if ( isNonEmptyIp || ( isIndefinite ) ) {
				$hideUser.goOut( instant );
			} else {
				$hideUser.goIn( instant );
			}
			if ( !isIp && !isEmpty ) {
				$anonOnlyRow.goOut( instant );
			} else {
				$anonOnlyRow.goIn( instant );
			}
			if ( isIpRange && !isEmpty ) {
				$watchUser.goOut( instant );
			} else {
				$watchUser.goIn( instant );
			}
		}

		if ( $blockTarget.length ) {
			// Bind functions so they're checked whenever stuff changes
			$blockTarget.keyup( updateBlockOptions );
			$expiry.change( updateBlockOptions );

			// Call them now to set initial state (ie. Special:Block/Foobar?wpBlockExpiry=2+hours)
			updateBlockOptions( /* instant= */ true );
		}
	} );
}( mediaWiki, jQuery ) );

Thats what im trying to, but for some reasons i cannot install gerrit and upload changes.

:(
I don't know what you have already tried yet; normally I'd recommend checking Developer Access to submit your proposed code changes as a Git branch directly into Gerrit for easier review and to provide feedback. If you run into specific issues, feel free to describe what you've tried and the potential problems on the corresponding talk page to receive feedback.
If you cannot or don't want to set up Git/Gerrit, you can also use the Gerrit Patch Uploader. Thanks again!

Lets do it like this: check if the code is ok

I can't as I'm not a developer. I'm also afraid that most developers prefer to check a diff which only displays the actual changed data. :)

EddieGP added a subscriber: EddieGP.EditedFeb 16 2017, 7:33 PM

Your code looks almost good to me. First let me give a diff of your code for everybody else who would be interested in just reading the changes:

diff --git a/resources/src/mediawiki.special/mediawiki.special.block.js b/resources/src/mediawiki.special/mediawiki.special.block.js
index aca335e..c4e2494 100644
--- a/resources/src/mediawiki.special/mediawiki.special.block.js
+++ b/resources/src/mediawiki.special/mediawiki.special.block.js
@@ -7,19 +7,26 @@
                        $anonOnlyRow = $( '#mw-input-wpHardBlock' ).closest( 'tr' ),
                        $enableAutoblockRow = $( '#mw-input-wpAutoBlock' ).closest( 'tr' ),
                        $hideUser = $( '#mw-input-wpHideUser' ).closest( 'tr' ),
-                       $watchUser = $( '#mw-input-wpWatch' ).closest( 'tr' );
+                       $watchUser = $( '#mw-input-wpWatch' ).closest( 'tr' ),
+                       $expiry = $( '#mw-input-wpExpiry' );
 
                function updateBlockOptions( instant ) {
                        var blocktarget = $.trim( $blockTarget.val() ),
                                isEmpty = blocktarget === '',
                                isIp = mw.util.isIPAddress( blocktarget, true ),
-                               isIpRange = isIp && blocktarget.match( /\/\d+$/ );
+                               isIpRange = isIp && blocktarget.match( /\/\d+$/ ),
+                               expiryValue = $expiry.val(),
+                               isIndefinite = expiryValue === 'infinite',
+                               isNonEmptyIp = isIp && !isEmpty;
 
-                       if ( isIp && !isEmpty ) {
+                       if ( isNonEmptyIp ) {
                                $enableAutoblockRow.goOut( instant );
-                               $hideUser.goOut( instant );
                        } else {
                                $enableAutoblockRow.goIn( instant );
+                       }
+                       if ( isNonEmptyIp || ( isIndefinite ) ) {
+                               $hideUser.goOut( instant );
+                       } else {
                                $hideUser.goIn( instant );
                        }
                        if ( !isIp && !isEmpty ) {
@@ -37,6 +44,7 @@
                if ( $blockTarget.length ) {
                        // Bind functions so they're checked whenever stuff changes
                        $blockTarget.keyup( updateBlockOptions );
+                       $expiry.change( updateBlockOptions );
 
                        // Call them now to set initial state (ie. Special:Block/Foobar?wpBlockExpiry=2+hours)
                        updateBlockOptions( /* instant= */ true );

}

When testing the feature two issues came up for me:

  1. It works the wrong way round: It hides the hideuser-field when I select infinite and shows it when I select something different
  2. It doesn't show the hideuser field when I select "other" from the dropdown box. The script should either
    • check if I typed something into 'other'-textfield that means 'infinite' (could be hard to determine this as mediawiki accepts different things here, at least 'infinite' and 'indefinite' work, I don't know what else does) or
    • simply also show the hideuser field when 'other' is selected on dropdown (which of course doesn't help when someone checks the hideuser box and types '5 hours' in the 'other'-field)

Number 1 is easy to solve, you just did use a boolean unnegated. Here's a diff using your code as a basis with what I've changed so that it works for me:

diff --git a/resources/src/mediawiki.special/mediawiki.special.block.js b/resources/src/mediawiki.special/mediawiki.special.block.js
index c4e2494..8174cc3 100644
--- a/resources/src/mediawiki.special/mediawiki.special.block.js
+++ b/resources/src/mediawiki.special/mediawiki.special.block.js
@@ -24,7 +24,7 @@
                        } else {
                                $enableAutoblockRow.goIn( instant );
                        }
-                       if ( isNonEmptyIp || ( isIndefinite ) ) {
+                       if ( isNonEmptyIp || !isIndefinite ) {
                                $hideUser.goOut( instant );
                        } else {
                                $hideUser.goIn( instant );

You may have noticed I've removed the inner paratheses, AFAIK those aren't necessary here and don't do anything, but I'm not really into JS and also just about to start developing, so if I'm wrong here you may correct me.

For number 2: From the dropdown menu the 'other' option seems to be the default, so when loading the page 'other' is selected and the 'other' input box is shown. Thus 'hideuser' would not be shown. I guess it is a ususal thing for some oversighters never to use the dropdown menu but to type everything out into the 'other' box. I would not like to break their workflow which is why I would recommend not to hide the hiddenuser-checkbox when 'other' is selected from the dropdown and it is not checked that the text in the 'other' input box is definitely something different from 'infinite' (by it's meaning, not by word, as I mentioned above).

Thanks for your patch and: Keep doing! I've also just started getting git and gerrit running, I know it's hard at the beginning. Following Gerrit/Tutorial on Wikitech step by step and google around while doing so to understand what I was doing really helped me. Feel free to ask if anything is unclear!

EddieGP triaged this task as Medium priority.Feb 16 2017, 7:42 PM

@EddieGP thank you very much for the review. I will do the changes. Im so sorry guys im making this harder for you. I will follow the gerrit tutorial and try it again, im used to use linux on my laptop and now that its broken im using an windows one. Worst case scenario i will be in a wiki event in March, a lot of Wikimedias who knows about gerrit will be there so i will ask for help there too. And everything will be fine with my other tasks :)

Nirmos added a subscriber: Nirmos.Feb 22 2017, 7:15 AM

could be hard to determine this as mediawiki accepts different things here, at least 'infinite' and 'indefinite' work, I don't know what else does

According to Special:ApiSandbox, the string "never" also works.

EddieGP moved this task from Backlog to Ready to go on the good first task board.Mar 15 2017, 5:06 PM

@Margott Any Progress on this or anything we could help you with?

@Margott Any Progress on this or anything we could help you with?

Hello @EddieGP, no after my last update on the code i haven't rechecked it. I have my finals at university till 28 March and no that much time left. If this task was suppose to be finished, please assign it to someone else. And i will be back to you after I set up the git environment properly for other tasks. If it can wait till last week of March, i would be more than happy to finish my first task before getting new ones.

Hey @Margott , everything is fine about waiting for this, just wanted to know if there's something we could help you with. But if it's not, just take your time and feel free to ask if you run into any trouble with this task (or setting up git).

But for now, just good luck for your exams ;)

Krinkle renamed this task from hideuser field on Special:block should be hidden with javascript when time field not indefinite to Field "hideuser" on Special:Block should be hidden when time field not indefinite.May 8 2017, 12:07 AM
Krinkle removed a project: MediaWiki-Special-pages.

Change 354486 had a related patch set uploaded (by EddieGP; owner: EddieGP):
[mediawiki/core@master] Hide "hideuser" on Special:Block for non-infinite blocks

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

As there hasn't been much activity on this lately I've been bold and picked up the patch from above, addressed the issues mentioned by myself, looked up what values are accepted for infinity (in the code, as this seems to be the most reliable source for this - the block class refers to the global function wfIsInfinity, the array there clarifies the allowed values) and uploaded the patch linked above. I hope that's okay to you, @Margott.

No problem :) thank you @EddieGP

For reference, there has been an older patch for this task at https://gerrit.wikimedia.org/r/#/c/318690/ (unfinished).

EddieGP claimed this task.May 21 2017, 1:18 AM
EddieGP added a subscriber: Margott.

For reference, there has been an older patch for this task at https://gerrit.wikimedia.org/r/#/c/318690/ (unfinished).

Hmm, sorry about that one, neither the bot nor anyone else mentioned it here, in which case I'd have picked up that one instead. Anyway, thanks for the reference. :-)

Krinkle closed this task as Resolved.Jul 29 2017, 4:16 AM
Krinkle removed a project: Patch-For-Review.

Change 354486 merged by jenkins-bot:
[mediawiki/core@master] Hide "hideuser" on Special:Block for non-infinite blocks

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