WordPress.org

Make WordPress Core

Opened 17 months ago

Closed 9 months ago

Last modified 4 months ago

#34521 closed task (blessed) (fixed)

Unifying permission error messages

Reported by: ramiy Owned by: ocean90
Milestone: 4.6 Priority: normal
Severity: normal Version:
Component: Text Changes Keywords: good-first-bug has-patch
Focuses: Cc:

Description

Ok, so we have many many many types of error messages. In this ticket I will try to fix/unify the use of the You are not allowed to ... strings.

See the strings:

As you can see in the links above, the majority of the strings are using the You are not allowed to ... prefix. But some strings use other prefixes:

  • You are not allowed to ... - the most popular prefix
  • Sorry, you are not allowed to ...
  • Sorry, you do not have the right to ...
  • Sorry, you cannot ...

I noticed that the Sorry, prefix is used only in class-wp-xmlrpc-server.php file. In the attached patch, I've removed the Sorry, prefix to create a unified error message changing and updating the different prefixes mentioned above.


Note: WordPress has many types of error messages, in this ticket I'm trying to unify the error messages that you usually see after the if ( ! current_user_can( 'xxx' ) ) check.

Attachments (21)

class-wp-xmlrpc-server.php.patch (25.1 KB) - added by ramiy 17 months ago.
class-wp-xmlrpc-server.php.2.patch (25.1 KB) - added by ramiy 17 months ago.
make this post sticky
class-wp-xmlrpc-server.php.3.patch (25.1 KB) - added by ramiy 17 months ago.
make this post private
pemission-messages-on-admin-network.patch (10.8 KB) - added by ramiy 17 months ago.
pemission-messages-on-wp-includes.patch (2.2 KB) - added by ramiy 17 months ago.
34521.patch (69.2 KB) - added by ramiy 17 months ago.
34521.2.patch (69.2 KB) - added by ramiy 14 months ago.
34521.3.patch (128.6 KB) - added by Presskopp 10 months ago.
34521.4.patch (72.7 KB) - added by Presskopp 10 months ago.
text-changes.png (776.4 KB) - added by ramiy 9 months ago.
Needed text changes
34521-text-change-import.3.patch (948 bytes) - added by ramiy 9 months ago.
Text change: import content
34521-text-change-export.patch (527 bytes) - added by ramiy 9 months ago.
Text change: export content
34521-text-change-bookmarks.patch (1.6 KB) - added by ramiy 9 months ago.
Text change: edit links
34521-text-change-manage-options.patch (600 bytes) - added by ramiy 9 months ago.
Text change: manage options
34521-text-change-manage-options.2.patch (589 bytes) - added by ramiy 9 months ago.
Text change: manage options
34521-text-change-add categories.patch (622 bytes) - added by ramiy 9 months ago.
Text change: add categories
34521-you-cannot-----you are not allowed.patch (6.2 KB) - added by ramiy 9 months ago.
"Sorry, you cannot" --> "Sorry, you are not allowed"
34521-you-can-not-----you are not allowed.patch (673 bytes) - added by ramiy 9 months ago.
"Sorry, you can not" --> "Sorry, you are not allowed"
34521-one-more-permission-string.patch (702 bytes) - added by ramiy 9 months ago.
update permission string
34521-text-change-manage-users.patch (662 bytes) - added by ramiy 9 months ago.
Text change: manage users - on XMLRPC blogger_getUserInfo() function
34521-text-change-manage-users.2.patch (666 bytes) - added by ramiy 9 months ago.
you are not allowed to access user data

Download all attachments as: .zip

Change History (79)

#1 @ocean90
17 months ago

  • Component changed from I18N to Text Changes

#2 @ramiy
17 months ago

  • Keywords has-patch 2nd-opinion added

Other error messages I found:

  • You don't have permission to ...
  • You do not have permission to ...
  • You do not have sufficient permissions to ...

Do you think we need to change all of them to You are not allowed to ... ???

#3 follow-up: @swissspidy
17 months ago

Just noticed that You are not allowed to stick this post. sounds weird. In the admin, core uses Make this post sticky.

You are not allowed to stick this post. (and similar) should probably be changed to You are not allowed to make this post sticky..

Do you think we need to change all of them to You are not allowed to … ?

Unifying these error messages seems like a good thing to me.

This ticket was mentioned in Slack in #core by ramiy. View the logs.


17 months ago

@ramiy
17 months ago

make this post sticky

#5 in reply to: ↑ 3 ; follow-up: @ramiy
17 months ago

Replying to swissspidy:

You are not allowed to stick this post. (and similar) should probably be changed to You are not allowed to make this post sticky..

Done!

Any more feedback?

#6 @ramiy
17 months ago

  • Summary changed from Translation strings - "You are not allowed to ..." to Unifying permission error messages

#7 in reply to: ↑ 5 ; follow-up: @swissspidy
17 months ago

Replying to ramiy:

Replying to swissspidy:

You are not allowed to stick this post. (and similar) should probably be changed to You are not allowed to make this post sticky..

Done!

Any more feedback?

There's also You are not allowed to stick a private post. with the same problem :) Probably other strings too, though I can't find any right now.

@ramiy
17 months ago

make this post private

#8 in reply to: ↑ 7 @ramiy
17 months ago

Replying to swissspidy:

There's also You are not allowed to stick a private post. with the same problem :) Probably other strings too, though I can't find any right now.

Done!

Didn't find any more stick strings.

#9 follow-up: @DrewAPicture
17 months ago

  • Keywords 2nd-opinion removed

I think I prefer "Sorry, you are not allowed to ..." over "You don't/do not have permission to ...".

For that matter, we should probably use "Sorry, you aren't allowed to ..." to match existing style elsewhere, see [34987] for more on that.

#10 in reply to: ↑ 9 @ramiy
17 months ago

Replying to DrewAPicture:

I think I prefer "Sorry, you are not allowed to ..." over "You don't/do not have permission to ...".

For that matter, we should probably use "Sorry, you aren't allowed to ..." to match existing style elsewhere, see [34987] for more on that.

Not sure about the Sorry part, only the class-wp-xmlrpc-server.php file uses this prefix, all the other files did not.

As for [34987], I changed 152 strings all over the core to use aren’t.

Last edited 16 months ago by ramiy (previous) (diff)

@ramiy
17 months ago

#11 @SergeyBiryukov
17 months ago

#14039 was marked as a duplicate.

This ticket was mentioned in Slack in #core by ramiy. View the logs.


17 months ago

#13 @ocean90
17 months ago

  • Keywords early added
  • Milestone changed from Awaiting Review to Future Release
  • Type changed from defect (bug) to enhancement

#14 @maor
15 months ago

+1.

I love this idea of standardizing the various error messages. Also, this is a personal opinion, but I do not like the wording for the kind of errors that begin with Sorry, .... Nothing to be sorry for ;)

#15 @nir_r
15 months ago

Thanks Rami for taking care of unifying the error messages...

#16 @Compute
15 months ago

+1 for the work Rami has done here! Hope this idea gets into core.

#17 @swissspidy
14 months ago

  • Milestone changed from Future Release to 4.5

#18 follow-up: @SergeyBiryukov
14 months ago

@DrewAPicture: Any objections to 34521.patch?

#19 in reply to: ↑ 18 ; follow-up: @DrewAPicture
14 months ago

Replying to SergeyBiryukov:

@DrewAPicture: Any objections to 34521.patch?

I like where this is going. In reviewing the strings I noticed a few minor issues:

  • This message in wp-admin/network/sites.php is strangely non-specific compared to most of the others: "You aren't allowed to delete the site."
  • Some of the IXR_Error strings don't end with periods, some do. Seems like maybe we should be consistent.
  • I notice a few strings include the mildly unhelpful phrase, "Something wrong happened." Not sure if we want to try fixing that here or separately

#20 in reply to: ↑ 19 @ramiy
14 months ago

Replying to DrewAPicture:

I like where this is going. In reviewing the strings I noticed a few minor issues:

  • This message in wp-admin/network/sites.php is strangely non-specific compared to most of the others: "You aren't allowed to delete the site."
  • Some of the IXR_Error strings don't end with periods, some do. Seems like maybe we should be consistent.
  • I notice a few strings include the mildly unhelpful phrase, "Something wrong happened." Not sure if we want to try fixing that here or separately

Thanks for the feedback Drew.

  • In this ticket I'm not trying change the strings meaning. Just to unify the prefix.
  • This is a very good point! I will add periods to all the strings.
  • I prefer to address this in a separate ticket.

@ramiy
14 months ago

#21 @SergeyBiryukov
14 months ago

  • Owner set to SergeyBiryukov
  • Status changed from new to reviewing

This ticket was mentioned in Slack in #core by chriscct7. View the logs.


14 months ago

#23 @adamsilverstein
14 months ago

@DrewAPicture & @SergeyBiryukov: Do you see any issues with this change? I'm assuming part of the goal here in standardizing the language is to make it easier on translators. This makes sense to me. Also, I'm not sure I like the encoded ampersands, i would almost go with 'are not' instead of aren't to avoid them (although I do see @drew commented on matching other language with that above). What do you think @ocean90?

Last edited 14 months ago by adamsilverstein (previous) (diff)

#24 @pavelevap
13 months ago

I like this change very much as a translator, but:

  • Current error messages are also well indexed for example by Google, users can struggle to debug new strings.
  • XML-RPC will be removed as a whole in the future so this problem mostly disappears at all?

This ticket was mentioned in Slack in #core by ocean90. View the logs.


13 months ago

#26 @ocean90
13 months ago

  • Keywords needs-refresh added; early removed

Current error messages are also well indexed for example by Google, users can struggle to debug new strings.

Since they will be still similar that's not really an issue.

XML-RPC will be removed as a whole in the future so this problem mostly disappears at all?

Not sure where you have read that, but there is no reason for removing it.


I did a quick poll on Slack at https://wordpress.slack.com/archives/core/p1455746910000403. 5 votes for "You are not allowed to edit this post." and 11 for "Sorry, you are not allowed to edit this post.". Since "Sorry, " is a pretty common pattern in core we should go with "Sorry, you are not allowed…". (Keep in mind that translators don't have to translate "Sorry".)

Last edited 13 months ago by ocean90 (previous) (diff)

#27 @pavelevap
13 months ago

You can check for example following search (Google):

  • You are not allowed to edit this page. - current message, many exact search results from wordpress.org or SE.
  • You aren’t allowed to edit this page. - proposed message, nothing relevant found.

Removing XML-RPC was mentioned probably somewhere in Trac (after adding REST API), but I am not sure about it, maybe it was only mentioned idea which was later denied.

What about adding translators comment, that these strings are wp_die() related? It could be helpfull for translators to know context.

This ticket was mentioned in Slack in #core by jorbin. View the logs.


13 months ago

#29 @jorbin
13 months ago

  • Type changed from enhancement to task (blessed)

#30 @ocean90
13 months ago

Is anyone working on a refresh?

#31 @ramiy
13 months ago

Guys, you are forgetting that those are not general error messages, it's only permission messages. They come after !current_user_can('xxx'), when the user don't have sufficient permissions to do something.

Maybe we should consider using:

  • You don't have permission to ...
  • You do not have permission to ...
  • You don't have sufficient permissions to ...
  • You do not have sufficient permissions to ...

#32 @ocean90
13 months ago

  • Keywords good-first-bug needs-patch added; has-patch needs-refresh removed
  • Milestone changed from 4.5 to Future Release
  • Type changed from task (blessed) to enhancement

Still needs a patch based on the decision from comment:26, punting for now.

@Presskopp
10 months ago

#33 @Presskopp
10 months ago

Refresh.

Please double-check with me, I just saw 2 mistakes :(

wp-admin/includes/file.php

wp-includes/deprecated.php

Last edited 10 months ago by Presskopp (previous) (diff)

#34 @ocean90
10 months ago

  • Keywords has-patch needs-refresh added; needs-patch removed

@Presskopp Thanks for the refresh!

34521.3.patch:

  • Please avoid any code styling changes like in wp-admin/custom-background.php.
  • Docs shouldn't be changed, see wp-admin/includes/file.php and wp-includes/deprecated.php.
  • "You cannot" shouldn't be replaced with "Sorry, you are not allowed". There is no way that you can perform the action. Let's leave these string as is.
  • In wp-includes/class-wp-xmlrpc-server.php "Sorry" is added twice: "Sorry, Sorry, …"

#35 @Presskopp
10 months ago

Ok, thank you, I will do these corrections..

@Presskopp
10 months ago

#36 @ocean90
9 months ago

  • Keywords needs-refresh removed
  • Milestone changed from Future Release to 4.6
  • Owner changed from SergeyBiryukov to ocean90
  • Status changed from reviewing to accepted

@Presskopp Please make sure that you always add a comment after uploading a patch since attachments don't trigger notifications.

#37 @ocean90
9 months ago

  • Resolution set to fixed
  • Status changed from accepted to closed

In 37914:

Text Changes: Unify permission error messages.

The new format looks like "Sorry, you are not allowed to <action>.". This provides a consistent experience for all error messages related to missing permissions. It also reduces the number of similar strings and allows translators to provide a consistent style in their language.

Props ramiy, Presskopp.
Fixes #34521.

#38 @SergeyBiryukov
9 months ago

In 37956:

Text Changes: Remove duplicate string, use the one we already have.

See #34521.

#39 @SergeyBiryukov
9 months ago

In 37958:

Text Changes: Remove duplicate string, use the one we already have.

See #34521.

#41 @SergeyBiryukov
9 months ago

In 37960:

Text Changes: Remove duplicate string, use the one we already have.

See #34521.

#42 @SergeyBiryukov
9 months ago

@ramiy: Thanks! I think the changesets above caught all the duplicates (in the scope of this ticket). If you find more, let me know :)

@ramiy
9 months ago

Needed text changes

#43 @ramiy
9 months ago

@SergeyBiryukov, the screenshot has few more changes.


I also think we should either remove the suffix or unify the suffix in those strings.

The current structure: Sorry, you are not allowed to <action><suffix>.

Action examples:

  • add posts/pages/comments
  • edit posts/pages/comments/themes/plugins
  • delete posts/pages/comments/themes/plugins
  • manage themes/plugins/options
  • install themes/plugins

Suffix examples (currently in use):

  • on this site
  • for this site
  • in this site
  • of this site
  • as this user

I personally think, in most case, we can ditch the suffix.

In other cases, where we use "to this network", we should not remove the suffix.

But we can definitely remove "as this user" suffix.

@ramiy
9 months ago

Text change: import content

@ramiy
9 months ago

Text change: export content

@ramiy
9 months ago

Text change: edit links

@ramiy
9 months ago

Text change: manage options

This ticket was mentioned in Slack in #core-i18n by ramiy. View the logs.


9 months ago

@ramiy
9 months ago

Text change: manage options

@ramiy
9 months ago

Text change: add categories

@ramiy
9 months ago

"Sorry, you cannot" --> "Sorry, you are not allowed"

#45 @ramiy
9 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Found 16 more strings.

"Sorry, you cannot..." --> "Sorry, you are not allowed..."

See my last patch.

@ramiy
9 months ago

"Sorry, you can not" --> "Sorry, you are not allowed"

@ramiy
9 months ago

update permission string

@ramiy
9 months ago

Text change: manage users - on XMLRPC blogger_getUserInfo() function

This ticket was mentioned in Slack in #core-i18n by ocean90. View the logs.


9 months ago

#47 @ocean90
9 months ago

In 37998:

Import: Merge two similar strings.

Props ramiy.
See #34521.

#48 @ocean90
9 months ago

Last edited 9 months ago by ocean90 (previous) (diff)

#49 @ocean90
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 37999:

Text Changes: Unify a few more permission error messages which were missed in [37914].

Props ramiy.
Fixes #34521.

@ramiy
9 months ago

you are not allowed to access user data

#50 follow-up: @ramiy
9 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Thank for the review @ocean90, I have a few notes:

Imports / Exports

For "imports" we are using "Sorry, you are not allowed to import content."

This is why the "export" should use "Sorry, you are not allowed to export content."

Patch: 34521-text-change-export.patch

Manage Users

The blogger_getUserInfo() function in wp-includes/class-wp-xmlrpc-server.php is checking for user permission using current_user_can() function. The patch 34521-text-change-manage-users.2.patch is fixing the permission error message using the new standard - "you are not allowed to...".

replacing "Sorry, you do not have access to user data on this site."

with this "Sorry, you are not allowed to access user data on this site."

Looks like a minor change, but it's what ticket is meant to do - minor changes all over the core.

API Permissions

As mentioned in the second comment, we replaced all the string saying:

  • You don't have permission to ...
  • You do not have permission to ...
  • You do not have sufficient permissions to ...

The rest-api is the last string using the old format.

34521-one-more-permission-string.patch is updating this string. The patch replacing the text and merging the string with other similar string - "Sorry, you are not allowed to do that.", it is used twice in wp-includes/script-loader.php.

#51 @ocean90
9 months ago

  • Type changed from enhancement to task (blessed)

#52 @ocean90
9 months ago

  • Resolution set to fixed
  • Status changed from reopened to closed

In 38037:

Text Changes: Unify/merge two more permission error messages.

Props ramiy.
Fixes #34521.

#53 in reply to: ↑ 50 @ocean90
9 months ago

Replying to ramiy:

For "imports" we are using "Sorry, you are not allowed to import content."

This is why the "export" should use "Sorry, you are not allowed to export content."

Patch: 34521-text-change-export.patch

IMO the current string reads much better than your proposal. I'd probably change the content string but let's call it done for 4.6.

#54 @SergeyBiryukov
6 months ago

In 38665:

Text Changes: Unify two permission error messages in wp-admin/network/site-users.php.

Props ramiy.
Fixes #38178. See #34521.

#55 @SergeyBiryukov
6 months ago

In 38666:

Text Changes: Unify two more permission error messages.

Props ramiy.
Fixes #38158. See #34521.

#56 @SergeyBiryukov
4 months ago

In 39251:

REST API: Unify permission error messages.

Props ramiy.
See #38791, #34521.

#57 @SergeyBiryukov
4 months ago

In 39253:

Text Changes: Unify permission error message in wp-ajax-response.js.

See #34521.

#58 @ramiy
4 months ago

More strings for WordPress 4.7 Rest API in #38803.

Note: See TracTickets for help on using tickets.