Make WordPress Core

Opened 14 years ago

Closed 9 years ago

Last modified 8 years ago

#14530 closed enhancement (fixed)

"Cheatin', uh?" is not helpful feedback for users or developers

Reported by: shidouhikari's profile shidouhikari Owned by:
Milestone: 4.4 Priority: normal
Severity: minor Version:
Component: Role/Capability Keywords: has-patch
Focuses: ui Cc:

Description

I've sen this infamous error msg more times than I wanted, in my own site where I'm admin.

That happens with more frequency when adding a new comment, but some times also inside admin pages.

I understand it's generally capability and permission tests that fail, and that happens in pages that ppl without permission to access them shouldn't see links to them, therefore they probably tried direct access to somewhere they shouldn't be going.

But also it happens when session expires or due to some bug. In this cases, the user is effectively not doing anything wrong, it may even be WP fault, and when that's the case the message is rude, even offensive to more emotional ppl.

Even worse, it gives no hint on what went wrong, so that user can try to fix it.

Then I suggest these messages to be changed, to more meaningful and also polite messages. Real cheaters and hackers will already have any info a message may provide, so a better explanation of what went wrong won't help them succeed in their attempt to hack a site, and will help a lot the victims of these errors.

Attachments (10)

chaetin.patch (13.9 KB) - added by mrmist 14 years ago.
Replace the cheatin message with a login message
14530-1.patch (13.6 KB) - added by kraftbj 12 years ago.
Change to You do not have sufficient permissions to access this page.
14530-2.patch (13.2 KB) - added by kraftbj 12 years ago.
Change to You do not have permission to view this page.
14530-3.patch (13.8 KB) - added by kraftbj 12 years ago.
Change to A permissions error occurred while attempting to access this page.
14530.diff (514 bytes) - added by kraftbj 10 years ago.
Update the error message for the Customizer
14530.2.diff (755 bytes) - added by ericlewis 10 years ago.
14530.2.png (98.9 KB) - added by ericlewis 10 years ago.
14530-4.patch (13.5 KB) - added by lukecarbis 10 years ago.
Change "Cheatin', uh?" to "You do not have permission to access this page."
14530.3.diff (18.1 KB) - added by ericlewis 10 years ago.
14530.4.diff (18.0 KB) - added by ericlewis 10 years ago.

Download all attachments as: .zip

Change History (99)

#1 @mrmist
14 years ago

  • Owner set to mrmist
  • Status changed from new to accepted

Not a problem I've come across, but I agree with the sentiment. I'll see if I can put something together for it.

#2 @nacin
14 years ago

This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug.

@mrmist
14 years ago

Replace the cheatin message with a login message

#3 @mrmist
14 years ago

  • Keywords has-patch added

Having checked this out, it's more difficult than I imagine to actually get one of these warnings. Nevertheless, on the off-chance that it's accessed by a valid user typing in a URL for a random page they don't have access to, attached patch with a more explanatory error.

#4 @markmcwilliams
14 years ago

Upon looking at the attached patch, would it be better to phrase the text as ... 'There was a problem loading this page, you may not have the necessary permissions, or may need to <a href="' . get_option('siteurl') . '/wp-login.php">' . __('login') . '</a> again?' ... so basically adding the again? after, more, if anything to make it make a little more sense?

#5 @mrmist
14 years ago

I did do it like that (with "again") originally, but on my screen it created word-wrap to the next line, which I didn't like, hence the way it is now. Trivial to do either way, really.

#6 @mrmist
14 years ago

  • Owner mrmist deleted
  • Status changed from accepted to assigned

#7 @nacin
14 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from assigned to closed

Per conversation in IRC from a few months ago, I'm going to close this as wontfix.

As I said, these are sanity checks. They're impossible to reach via the UI. (If they were, that would be a bug.) You need to hit a URL that you specifically don't have access to. You need to be logged in as well, so the link to wp-login is invalid.

#8 @kraftbj
12 years ago

  • Cc bk@… added
  • Resolution wontfix deleted
  • Status changed from closed to reopened

I'd like to request reconsideration of this ticket.

While rare, when encountered it gives an unprofessional appearance of WordPress. I had a contact from a perspective client who ditched their previous consultant primarily because, after seeing this error message, assumed the guy was doing something shady on his site.

I grant that for it to appear, the consultant was probably doing something wrong, but that isn't the point.

Despite the rarity, is there a reason that it should be kept at "Cheatin'" and not something else?

To recreate message:

  1. Log in to /wp-admin/ as privileged user (administrator, editor), etc. Leave that tab alone.
  2. In separate tab, visit /wp-login.php and login as a Subscriber user.
  3. In original tab, visit the Categories or Tags link (Posts->Tags, etc).

That workflow isn't common, but I could foresee someone in IT showing off a feature of WordPress to someone on the business side of a company and getting that message after mistakenly switching to the wrong tab/not re-logging in first/etc.

Three ideas:

  1. Use the standard permissions error message ("You do not have sufficient permissions to access this page.") and reduce a string for the polyglots.
  2. Use something slightly different to keep it unique to the cheater check. ("You do not have permission to view this page.")
  3. Something more unique ("A permissions error occurred while attempting to access this page.") to help polyglots keep it different.

Patches forthcoming for the three options.

tl;dr -- Sterilize the error a bit to make it more palatable to folks who don't understand our sense of humor. Nothing more needed.

Last edited 12 years ago by kraftbj (previous) (diff)

@kraftbj
12 years ago

Change to You do not have sufficient permissions to access this page.

@kraftbj
12 years ago

Change to You do not have permission to view this page.

@kraftbj
12 years ago

Change to A permissions error occurred while attempting to access this page.

#9 @travisnorthcutt
12 years ago

  • Cc travis@… added

+1 on this. I don't see any particular reason not to replace the (snarky, IMO) current message with something more explicit about what's going on.

#10 @SergeyBiryukov
12 years ago

  • Milestone set to Awaiting Review

#11 @johnbillion
12 years ago

This message would also benefit from having a link back to either the site home page, or the admin dashboard, so the message isn't such a dead end.

#12 @kraftbj
12 years ago

John-- I think you're right, but I think we should do that under a different ticket. The other "permission errors" do not include a link back, so my two cents is let's get the text changed, and focus another ticket on the link back.

A quick look at http://core.trac.wordpress.org/browser/tags/3.5.1/wp-includes/functions.php#L2049 tells me there is a 'back_link' ability, so, without knowing the code better, may be another fix that is more proper than changing the text string.

#13 @Dorian Speed
12 years ago

  • Cc Dorian Speed added

I agree - I have been in the same situation of having a frustrated client encounter that message and assume there's something shady going on with his website. I like "A permissions error occurred while attempting to access this page."

#14 @kraftbj
12 years ago

FWIW, I'm helping out over on the WordPress.com forums and an user had the Cheatin' message appear. I think it was a similar reason as described in Comment 8.

http://en.forums.wordpress.com/topic/i-want-my-url-to-be-thisonetreewordpresscom-its-attached-to-my-other-email?replies=13#post-1257253

Despite the rarity, I think a more descriptive message would be a better UX.

#15 follow-up: @tzkmx
11 years ago

Hi, I'm getting this error "message" trying to activate some child themes. The funny part is they are all child of the same parent theme, and just one of the child themes loads in the customize screen, all others die into the "Cheatin' uh?" "message".

Same user, same permissions. The only bit I found wp_die function in customize.php is this:

if ( ! current_user_can( 'edit_theme_options' ) )
        wp_die( __( 'Cheatin&#8217; uh?' ) );

So, why is this? Have I different permissions according to the theme? Is there only one child theme allowed for any parent theme in a given WP installation?

FWIW, we are trying to setup several child themes for the same parent theme in network aimed to teach a few people how to extend WP themes without messing other people work.

#16 in reply to: ↑ 15 @tzkmx
11 years ago

Replying to tzkmx:

Same user, same permissions. The only bit I found wp_die function in customize.php is this:

if ( ! current_user_can( 'edit_theme_options' ) )
        wp_die( __( 'Cheatin&#8217; uh?' ) );

My bad, we were creating the stylesheets and template names were correct, but the child themes we couldn't activate, were those we haven't activated for the network yet. And we were seeing anyway those themes listed on every site, because a function on Hyper Admins plugin listed it.

I'd like to propose a patch to split the error message on permission, not only on edit_theme_options capability, but checking if the theme is enabled and giving the user the advice to enable for the site first. However I don't know which function could be used, if I discover a way to accomplish this, I'd be more helpful on this thread than just complaining about an uninformative error message.

#18 @kraftbj
10 years ago

I'm going to mostly flip-flop on this ticket.

I'm okay wontfix'ing it. The error is really difficult to throw except in two situations:

  1. Edit Tags/Categories
  2. Customizer

In instance 1, it should be throwing "You do not have sufficient permissions to access this page" ( https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/menu.php#L317 ), except $pagenow includes the query string (e.g. edit-tags.php?taxonomoy=category) while the $*_nopriv includes only edit-tags.php. (ref: https://core.trac.wordpress.org/browser/trunk/src/wp-admin/includes/plugin.php#L1637 ).

That is the root issue that should be addresses, IMO.

Instance 2: The Customizer doesn't use the admin menu, so it skips the check that is "misfiring" from instance 1. Changing the error text for the Customizer would be nice.

Attaching a patch to only change for the Customizer.

@kraftbj
10 years ago

Update the error message for the Customizer

#19 @SergeyBiryukov
10 years ago

#29253 was marked as a duplicate.

#20 @guidod
10 years ago

This happens often when an active user gets its permissions limited while inside the backend. A simple refresh or menu click results in a "Cheating, huh?" message, which is indeed rude and confusing for typical users, when it should actually read something like:

"You don't have permission to access this page. If you think this is a mistake, contact your administrator. [link to dashboard]"

#21 @ericlewis
10 years ago

  • Focuses ui accessibility added
  • Keywords needs-patch added; has-patch removed
  • Type changed from defect (bug) to enhancement

The ability to debug is hampered with such a cryptic, blanket message for permission errors, as well as users understanding the software that they're using.

We should state clearly what the lacking capability. i.e. "Your user account does not have permission to edit theme options." "Your user account does not have permission to edit terms."

This ticket was mentioned in IRC in #wordpress-dev by ericandrewlewis. View the logs.


10 years ago

This ticket was mentioned in Slack in #forums by ceo. View the logs.


10 years ago

This ticket was mentioned in Slack in #forums by ceo. View the logs.


10 years ago

#25 @ericlewis
10 years ago

  • Milestone changed from Awaiting Review to 4.2

Let's get after this.

#26 @ericlewis
10 years ago

  • Summary changed from Cheating huh? to "Cheatin' huh?" is not helpful feedback for users or developers

#27 @ericlewis
10 years ago

  • Summary changed from "Cheatin' huh?" is not helpful feedback for users or developers to "Cheatin', uh?" is not helpful feedback for users or developers

#28 @ericlewis
10 years ago

Related: #30798 to ponder other wp_die() messages usefulness.

#29 follow-up: @valendesigns
10 years ago

Could we possibly add a server log message, so that at the very least we can debug the issue as it arises? I must say that this is more common of a page to see than some of you are saying. Especially when you're developing as a CMS and your CPT's & taxonomies are still in flux, as an example. I propose we do the following.

  1. Switch to a more friendly message for all instances of "Cheatin' huh?".
  2. Add detailed server log, or _doing_it_wrong, messages where applicable.

#30 @joedolson
10 years ago

  • Focuses accessibility removed

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


10 years ago

#32 in reply to: ↑ 29 @ericlewis
10 years ago

Replying to valendesigns:

Could we possibly add a server log message, so that at the very least we can debug the issue as it arises?

I'd say let's wait a little to see if #28319 produces a more robust method for logging application-level errors.

@ericlewis
10 years ago

@ericlewis
10 years ago

#33 @ericlewis
10 years ago

In attachment:14530.2.diff, a prototype of a more helpful message.

As this is typically a developer-facing error, this will assist in debugging.

If a user ends up seeing this message, this may still be helpful, helping them realize they're dealing with a permissions error which is in their control. At the very least, it is not cheeky/unprofessional.

I include the exact file and line number in loving memory of my previous debug sessions against this screen.

#34 follow-up: @helen
10 years ago

Acknowledging that you labeled it a prototype, no full path disclosure, please. :)

#35 in reply to: ↑ 34 @ericlewis
10 years ago

Replying to helen:

Acknowledging that you labeled it a prototype, no full path disclosure, please. :)

Good point - how about ABSPATH-relative? e.g. wp-admin/edit-comments.php

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


10 years ago

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


10 years ago

#38 follow-ups: @jorbin
10 years ago

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application. I do think that there is a useful point that we can make this experience better for developers though, so I think we should create a standard filter and pass in the apporiate things so that a message like the one eric is proposing is possible. Essentially, something like this:

$user = wp_get_current_user(); 
$message = apply_filter( 'no_go_message', 'Cheatin&#8217; uh?', $user, $failed_cap, __FILE__ );
wp_die( $message, 403 );

Then, much like we have the debug bar plugin, someone could create a debug access plugin that turned the message into one that works for developing while not changing the flavor of WordPress.

#39 follow-up: @helen
10 years ago

I wonder if we could add a clearer error message underneath when WP_DEBUG is on.

#40 in reply to: ↑ 38 ; follow-up: @guidod
10 years ago

The usefulness of a better error message is not just for developers, it's for end users as well. For example, a suscriber or editor that has a permission removed while he is browsing the admin panel would now get "Cheating, huh?" instead of something helpful like "You don't have permission to access this page, perhaps your permissions have changed?".

(And about the stuffy corporate application comment, WordPress IS used in corporate contexts).

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application. I do think that there is a useful point that we can make this experience better for developers though, so I think we should create a standard filter and pass in the apporiate things so that a message like the one eric is proposing is possible. Essentially, something like this:

$user = wp_get_current_user(); 
$message = apply_filter( 'no_go_message', 'Cheatin&#8217; uh?', $user, $failed_cap, __FILE__ );
wp_die( $message, 403 );

Then, much like we have the debug bar plugin, someone could create a debug access plugin that turned the message into one that works for developing while not changing the flavor of WordPress.

#41 in reply to: ↑ 38 ; follow-up: @ericlewis
10 years ago

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application.

If we want to sustain the flavor, let's insert a cheeky "something's messed up" image. As guidod notes, this is often a user-facing error, so surfacing a helpful error message can help users solve a capability issue - e.g. "oh, this tells us Jenn doesn't have the capability edit_theme_options. Let's elevate her role to Administrator."

#42 in reply to: ↑ 41 @jorbin
10 years ago

Replying to ericlewis:

"oh, this tells us Jenn doesn't have the capability edit_theme_options. Let's elevate her role to Administrator."

A filter that you can then choose to log or trigger a message to an admin, or send an event into graphite, etc would enable you to solve this. Changing the message wouldn't.

#43 in reply to: ↑ 40 ; follow-up: @jorbin
10 years ago

Replying to guidod:

The usefulness of a better error message is not just for developers, it's for end users as well. For example, a suscriber or editor that has a permission removed while he is browsing the admin panel would now get "Cheating, huh?" instead of something helpful like "You don't have permission to access this page, perhaps your permissions have changed?".


As was pointed out in the second comment of this thread. "This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug."

(And about the stuffy corporate application comment, WordPress IS used in corporate contexts).

It is, and we haven't needed to lose the personality of WordPress to do so.

#44 in reply to: ↑ 43 @ericlewis
10 years ago

Replying to jorbin:

As was pointed out in the second comment of this thread. "This warning should never be accessible via the UI. These are nothing more than sanity checks. If they can be accessed in a normal setup via the UI then that is a bug."

[In an imaginary organization's Slack]
Jenn: How do I edit the header image on our website?
Tom: Go here: http://site.com/wp-admin/customize.php
Jenn: Uh that page just says "Cheatin', uh?" Do you know what that means?

#45 in reply to: ↑ 39 @rmccue
10 years ago

Replying to helen:

I wonder if we could add a clearer error message underneath when WP_DEBUG is on.

This is my exact thought, since we could handle this without losing the personality here.

Something like...

if ( WP_DEBUG ) {
    $message .= '<p>' . sprintf( __( "Psst, <code>%s</code> does not have the <code>%s</code> capability." ), $user, $cap ) . '</p>';
}

Alternatively, we could also do it in a less specific way for all users:

$message .= '<p>' . __( "Psst, you don't have permission to access this page." ) . '</p>';

I'm definitely -1 on showing the actual permission name though for non-debug, as we don't expose these anywhere (roles are our user-facing abstraction for these).

#46 follow-up: @jorbin
10 years ago

I like the idea of the improved message with WP_DEBUG enabled, but the more I think about it, the more I think this is an event that should be capturable since as has been pointed out multiple times, this message should never be seen by a user and if they are seeing it, it is a bug. Making this capturable would allow a logger (such as the one proposed in #30934 ) to make this bug one a developer could track down and fix.

rmccue is absolutely correct, we don't expose capabilities in the UI anywhere, I don't think we should start.

#47 in reply to: ↑ 46 @guidod
10 years ago

If they can be accessed in a normal setup via the UI then that is a bug

By that logic, 404s are bugs too, but that doesn't mean they shouldn't be translated to a friendly message to end users. Otherwise, let's replace the template_redirection to 404.php with a "Cheating, uh?" as well.

This should NOT be a debug message. As Eric Lewis points out, this is required information for anyone that follows a URL, either bookmarked or sent by another user of the WordPress instance, that he can't access. This is, in fact, unlikely to happen on development and debug instances; it confuses users on PRODUCTION sites.

Replying to jorbin:

rmccue is absolutely correct, we don't expose capabilities in the UI anywhere, I don't think we should start.

But we expose roles, and users. If the role field isn't called "Uh huh foo-bar" then the role/permissions-related error message shouldn't read "Cheating, uh?".

#48 @SergeyBiryukov
10 years ago

Replying to jorbin:

"Cheatin', uh?" to me, is a core part of the flavor of WordPress. I wouldn't want to lose it and become a stuffy corporate application.

I don't necessarily disagree, but want to point out that I received some angry emails over the years via Rosetta contact form from users who thought it was either an inappropriate translation or a core bug.

this message should never be seen by a user and if they are seeing it, it is a bug.

Well, apparently it can be seen by a user in certain situations, whether it's a core or plugin/theme bug, so making it less confusing and more helpful would be great.

#49 @DrewAPicture
10 years ago

  • Severity changed from normal to minor

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


10 years ago

#51 @lukecarbis
10 years ago

  • Keywords has-patch added; needs-patch removed

To summarise,

  • While "Cheatin', uh?" is part of the flavour of WordPress, there are scenarios in which it confuses users and seems unprofessional. (comment:18) (comment:48)
  • Debug information (such as a user's capability) should not be displayed when WP_DEBUG is false. (comment:45)
  • Neither should debug information be displayed when WP_DEBUG is true - this should be handled elsewhere. (comment:46) (#30934)

I'm in favour of not introducing a new string for our translators. We already have a string which fits perfectly well, and says the same thing as most suggestions in this thread.

"You do not have permission to access this page."

Last edited 10 years ago by lukecarbis (previous) (diff)

@lukecarbis
10 years ago

Change "Cheatin', uh?" to "You do not have permission to access this page."

@ericlewis
10 years ago

#52 @ericlewis
10 years ago

@lukecarbis - thanks for the patch, however, we decided direction on this ticket in core chat last week.

attachment:14530.3.diff is a minimum viable change here. Use generic text (e.g. "item," "items") in places where the item in question is variable.

@ericlewis
10 years ago

#53 @ericlewis
10 years ago

attachment:14530.4.diff removes some stray sprintf()s

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


10 years ago

#55 @helen
10 years ago

  • Milestone changed from 4.2 to Future Release

Seems like we lost track of this a bit in 4.2 - this is a lot of new strings to introduce, and I'm not sure we need to get quite so specific in the user-facing language. As I noted before, we could perhaps show more details with WP_DEBUG on. I am (sadly) punting this - let's maybe hash on some possible language again before spending time re-patching.

#56 @jorbin
9 years ago

  • Keywords needs-refresh added
  • Milestone changed from Future Release to 4.4

Let's get this finished up. I think the patch @EricLewis worked on is a great start. It needs to be updated so it applies cleanly. Some of the language may need some minor tweaks, but it strikes a good balance of succulence and simplicity.

Perhaps we should do file specific patches so that if we need to debate wording on one or two, the entire ticket isn't blocked (and thus the patch goes stale)?

I agree with Helen that if WP_DEBUG is enabled, we can always provide more information.

#57 follow-up: @kraftbj
9 years ago

Logistical question: Is there a good way to manage 18 separate patches (2 more files since the last patch use Cheatin), all possibly in flux? Alternatively, we could follow the inline docs approach of opening tickets for each file (or group of files) and use this one as a master thread.

For 4.4, how about replace the messaging as first priority. Stretch goal use WP_DEBUG for additional info, but can punt that to 4.5+ if it puts anything at risk for 4.4.

#58 in reply to: ↑ 57 @SergeyBiryukov
9 years ago

Replying to kraftbj:

Alternatively, we could follow the inline docs approach of opening tickets for each file (or group of files) and use this one as a master thread.

Let's do that.

#59 @kraftbj
9 years ago

Spin off tickets, all in wp-admin, unless noted:
#33667: custom-header.php
#33668: customize.php
#33669: edit-comments.php
#33670: edit-tags.php
#33671: edit.php
#33672: media-upload.php
#33673: nav-menus.php
#33674: options.php
#33675: post-new.php
#33676: press-this.php
#33677: themes.php
#33678: user-new.php
#33679: users.php
#33680: widgets.php
#33682: includes/bookmark.php
#33683: network/site-users.php
#33684: wp-includes/class-wp-customize.manager.php
#33685: wp-includes/script-loader.php

@helen and I are aiming to slam through migrating/writing patches later today. We'll be in #core :)

#60 follow-up: @pavelevap
9 years ago

I saw the patch and it should respect currently existing strings (or merge them).

Example:

Suggested string in patch: "You are not allowed to edit posts."
But we already have: "Sorry, you are not allowed to edit posts."

We can use string which is available or merge it with better suggestion. But it is necessary to check all new strings against existed strings to avoid duplication of similar strings.

#61 in reply to: ↑ 60 @kraftbj
9 years ago

Replying to pavelevap:

I saw the patch and it should respect currently existing strings (or merge them).

We can use string which is available or merge it with better suggestion. But it is necessary to check all new strings against existed strings to avoid duplication of similar strings.

Good point. I moved over patches up to and including #33672 already. I'll be mindful of that as I move over the rest and we can circle back on the ones already moved over to improve the strings used (both to merge with existing and improve the language as needed).

Last edited 9 years ago by kraftbj (previous) (diff)

#62 @SergeyBiryukov
9 years ago

In 33852:

Provide more helful feedback than just “Cheatin’ uh?” for permission errors in wp-admin/edit-tags.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33670. see #14530.

#63 @SergeyBiryukov
9 years ago

In 33853:

Provide more helful feedback than just "Cheatin' uh?" for permission errors in wp-admin/media-upload.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33672. see #14530.

#64 @SergeyBiryukov
9 years ago

In 33854:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/custom-header.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33667. see #14530.

#65 @SergeyBiryukov
9 years ago

In 33857:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/customize.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33668. see #14530.

#66 @SergeyBiryukov
9 years ago

In 33860:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/edit-comments.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33669. see #14530.

#67 @SergeyBiryukov
9 years ago

In 33861:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/edit.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33671. see #14530.

#68 @SergeyBiryukov
9 years ago

In 33862:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/nav-menus.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33673. see #14530.

#69 @SergeyBiryukov
9 years ago

In 33863:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/options.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33674. see #14530.

#70 @SergeyBiryukov
9 years ago

In 33864:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/post-new.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33675. see #14530.

#71 @SergeyBiryukov
9 years ago

In 33865:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/press-this.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33676. see #14530.

#72 @SergeyBiryukov
9 years ago

In 33866:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/themes.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33677. see #14530.

#73 @kraftbj
9 years ago

  • Keywords needs-refresh removed

Patches on all break out tickets.

#74 follow-up: @juliobox
9 years ago

  • Keywords needs-refresh added

Hello

I'm wondering why do we had so many strings to translate and why do we explain more each security message.
Yes i talk about security messages because if someone reach a "Cheatin uh" message, this is because he hacked something to try to do something he couldn't, following me?

So, look at this one #21076, we deprecated the function wp_explain_nonce because it was not secure to add a too specific message, and also there were too many strings to translate for not so usefull feature.

Now check this one #33655, it sounds a little like this one too, but for the 14530 it's ok. I mean, if we really want to display the new messages, ok, but why all in one patch? Why not one by one when someone touche something related to, like when we do coding standards.

Thanks!

#75 in reply to: ↑ 74 @kraftbj
9 years ago

Replying to juliobox:

Hello

I'm wondering why do we had so many strings to translate and why do we explain more each security message.
Yes i talk about security messages because if someone reach a "Cheatin uh" message, this is because he hacked something to try to do something he couldn't, following me?

While rare, there are legit situations where "Cheatin" revealed itself. Example in this testing steps here: https://core.trac.wordpress.org/ticket/33684#comment:1

So, look at this one #21076, we deprecated the function wp_explain_nonce because it was not secure to add a too specific message, and also there were too many strings to translate for not so usefull feature.

For the breakout patches, I tried to reuse strings as much as possible and keep them semi-useful, semi-vague. The idea is to have more specific messaging returned when WP_DEBUG is enabled to help developers if they built something that stumbled upon one of these calls.

Now check this one #33655, it sounds a little like this one too, but for the 14530 it's ok. I mean, if we really want to display the new messages, ok, but why all in one patch? Why not one by one when someone touche something related to, like when we do coding standards.

Core has said that refactoring for the sake of coding standards is not something we'd do. In this case, IMO, we're looking at enhancing the user experience by providing something more useful for an error message besides simply "Cheatin' Uh?". Huge enhancement? I admit, no. :)

#76 @kraftbj
9 years ago

  • Keywords needs-refresh removed

#77 @SergeyBiryukov
9 years ago

In 33884:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/user-new.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33678. see #14530.

#78 @SergeyBiryukov
9 years ago

In 33885:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/users.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33679. see #14530.

#79 @SergeyBiryukov
9 years ago

In 33886:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/widgets.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33680. see #14530.

#80 @SergeyBiryukov
9 years ago

In 33887:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/includes/bookmark.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33682. see #14530.

#81 @SergeyBiryukov
9 years ago

In 33888:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/network/site-users.php.

props ericlewis, kraftbj, lukecarbis, mrmist.
fixes #33683. see #14530.

#82 @SergeyBiryukov
9 years ago

In 33889:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-includes/class-wp-customize-manager.php.

props kraftbj.
fixes #33684. see #14530.

#83 @paulwilde
9 years ago

Out of interest why not create a separate wp_die() function which wraps the heading and accepts the paragraph text as an argument? It would reduce a lot of repetition and then the upside would be that if things need to be expanded/altered in the future it's just one function to alter rather than multiple files.

For example, if the heading of "Cheatin&#8217; uh?" were to be changed in the future, that's a lot of file changes. If everything was wrapped in a single function that's a 1 line change.

It also has the added benefit that third-party plugins could then adopt the new function with the knowledge that it with be consistent with how WordPress is handling permission notices.

Last edited 9 years ago by paulwilde (previous) (diff)

#84 @kraftbj
9 years ago

@paulwilde - wp_die() needs a little bit of love, IMO. I overlooked it until after all of these patches that wp_die accepts a number of args— $message, $title, $args. It is set so if an int is used for title or args, it'll use that as the HTTP response code (or it could be set within the $args array). I haven't seen the $title arg being used before—only response code as the second arg—so I made a poor assumption on what it could handle.

Currently $title is only outputted as the html title of the wp_die response page. I think we could expand wp_die to output the $title as an <h1> and the $message within a <p> to mimic how these patches did it.

The immediate goal of this ticket was to give context to the Cheatin' message. There is so much randomness in how wp_die is used within Core. There's plenty of permission-style checks that do not include the Cheatin' message at all (and I'm not suggesting they begin to now). Even though I discovered it after the fact, I'm still behind this ticket resolving as it did to get it shipped, to be frank. It's been open five years and if this method got enough thumbs up to move forward, I'm all about taking it and iterating tomorrow. :)

I haven't looked for or opened tickets yet, but I think we can 1. discuss updating wp_die to output the title within the body. 2. "fix" these tickets to use the title arg of wp_die instead of manually indicating html and 3. audit the general use of wp_die (within core, plugins) to attempt to unify up the messaging, reduce strings, use wp_die's backlink function when applicable, etc.

I'm going to poke around a bit more before making an actual proposal to attempt to ensure we're not missing anything and not encouraging increased usage of wp_die itself (as code should handle errors as gracefully as possible as often as possible).

#85 @SergeyBiryukov
9 years ago

In 33902:

Provide more helpful feedback than just "Cheatin' uh?" for permission errors in wp-admin/js/customize-controls.js.

fixes #33685. see #14530.

#86 @SergeyBiryukov
9 years ago

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

Closing as fixed with the commits above, let's open new tickets for any new issues or suggestions.

Thanks everyone!

#87 @ramiy
8 years ago

Related: #37492

#88 @Presskopp
8 years ago

#38332 was marked as a duplicate.

#89 @BandonRandon
8 years ago

#38332 was marked as a duplicate.

Note: See TracTickets for help on using tickets.