Page MenuHomePhabricator

Block list and log incorrectly report "cannot edit own talk page" for some blocks
Closed, ResolvedPublic3 Estimated Story Points

Description

Partial blocks usually ignore the "Editing their own talk page" checkbox on Special:Block (see https://www.mediawiki.org/wiki/Help:Blocking_users#Blocking for full details), so this checkbox is normally disabled for partial blocks.

Users without JS, and users creating blocks via the API, can choose this option with partial blocks, but it won't be enforced. However, Special:BlockList, Special:Log/block etc. will incorrectly report "cannot edit own talk page" in these situations. (See also T224032#5210078)

These lists should be consistent with how the block is enforced, not how it was created.

Acceptance criteria
ipb_allow_usertalk should be saved as false only when:

  • A block is sitewide
  • A block is partial and there is a restriction on the User_talk namespace

If a user tries to save ipb_allow_usertalk in a way that does not conform with those rules, an error message should be shown and the block should not be saved.

Event Timeline

Assuming this task is about the user management in MediaWiki core, hence adding MediaWiki-User-management so someone could find this task.

Niharika triaged this task as Medium priority.Jun 12 2019, 11:40 PM
Niharika moved this task from Untriaged to Triage/To be Estimated on the Anti-Harassment board.

If a user tries to save ipb_allow_usertalk in a way that does not conform with those rules, an error message should be shown and the block should not be saved.

Or maybe just do the thing that is expected, rather than throwing an error (because if the form field is in a disabled state, you wont be able to resolve the error).

In other words, DWIM.

If a user tries to save ipb_allow_usertalk in a way that does not conform with those rules, an error message should be shown and the block should not be saved.

Or maybe just do the thing that is expected, rather than throwing an error (because if the form field is in a disabled state, you wont be able to resolve the error).

In other words, DWIM.

I think that in this particular case admins could interpret DWIM as a bug. They'll be sending "prevent edit their own talk page" and when they get the block back the field will be different. I don't think we should go that route with this.

Agreed with @dmaza. Users might interpret the auto-correction as a bug. By showing them the error we also educate them about what's allowed and what's not and why.

I'm saying you can show the error message, but the field may be disabled, so they may not be able to correct the error.

Change 526292 had a related patch set uploaded (by Dmaza; owner: Dmaza):
[mediawiki/core@master] [WIP] Fix SpecialBlock validation for ipb_allow_usertalk

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

I'm saying you can show the error message, but the field may be disabled, so they may not be able to correct the error.

This issue only arise when Js is not enabled or when using the API. As far as I can tell this field is only disabled via Js.
We have also implemented client side logic that will uncheck disabled fields on this form so I don't see the scenario you are mentioning happening unless I'm missing something.

Regarding the error message, I was thinking something like Editing their own talk page can only be blocked for sitewide blocks or partial blocks with restrictions on the User Talk namespace. Let me know what you think.

Regarding the error message, I was thinking something like Editing their own talk page can only be blocked for sitewide blocks or partial blocks with restrictions on the User Talk namespace. Let me know what you think.

Works for me.

Change 526292 merged by jenkins-bot:
[mediawiki/core@master] Fix SpecialBlock validation for ipb_allow_usertalk

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

@dmaza Would it be better if the error message appeared underneath the "Editing their own talk page" checkbox?

This would make it immediately obvious what it refers to and would make it consistent with the location of other error messages (see below).

Also, the error message does not appear if there are one or more other validation errors (see below).

specialblock_error_messages.png (814×762 px, 36 KB)

@dmaza Would it be better if the error message appeared underneath the "Editing their own talk page" checkbox?

This would make it immediately obvious what it refers to and would make it consistent with the location of other error messages (see below).

Makes sense, I'll fix it

Also, the error message does not appear if there are one or more other validation errors (see below).

For the error messages displayed on top of the form that's how the page behaves, It shows one at a time but I'll double check

@dom_walden can you confirm that the issue is fixed tho? (Block list and log incorrectly report "cannot edit own talk page" for some blocks) I'll take a look at how the form displays the errors anyway

@dom_walden can you confirm that the issue is fixed tho? (Block list and log incorrectly report "cannot edit own talk page" for some blocks) I'll take a look at how the form displays the errors anyway

The issue in the Description? Yes. Both via the API and Special:Block.

The error messages are fairly inconsistently located on Special:Block... These error messages are also located at the top (and only the first one encountered will be displayed):

  • already blocked
  • hide user with the wrong settings (e.g. with finite duration)
  • entering a time in the past in the expiry widget text input

As pointed out on the commit, fixing these is a bit fiddly, since the API doesn't trigger the HTML form validation. I'm not sure if it's worth fixing some but not all, since the UI will still be inconsistent...

I'd advocate for fixing all at once if there's a clean way to do it. @dmaza did you have a way in mind?

@Niharika Do we know how much use the PHP form gets?

I'd advocate for fixing all at once if there's a clean way to do it. @dmaza did you have a way in mind?

I do not.

I agree that changing the location of this message alone doesn't make sense so I'll abandon the above patch.
With that said, the issue on this task was resolved by rMWfd70b59dc5ae: Fix SpecialBlock validation for ipb_allow_usertalk so I think we can mark it as resolved. @Niharika, Is that ok with you?

I agree that changing the location of this message alone doesn't make sense so I'll abandon the above patch.
With that said, the issue on this task was resolved by rMWfd70b59dc5ae: Fix SpecialBlock validation for ipb_allow_usertalk so I think we can mark it as resolved. @Niharika, Is that ok with you?

That's okay with me.