WordPress.org

Make WordPress Core

Opened 8 years ago

Closed 8 years ago

Last modified 7 years ago

#2734 closed defect (bug) (fixed)

Nonce confirmation not informative.

Reported by: mdawaffe Owned by:
Milestone: Priority: normal
Severity: normal Version: 2.0.3
Component: Administration Keywords: nonce bg|has-patch bg|2nd-opinion
Focuses: Cc:

Description

The current confirmation message is just a Yes/No without any information about what's happening. Trying to write code that would tell the user exactly what's happening in this dialog would be expensive especially considering that this dialog will only rarely be seen.

We can, however, give some context.

The attached includes admin-header.php (and admin-footer.php) in the confirmation screen and tweaks the menu and submenu identifiers in a few files so that the confirmation is always displayed with the right menu highlighting and the right submenu.

The patch calls a lot more code and a lot more hooks than the current dialog does, which may be something to think about with regard to security.

Attachments (2)

2734.diff (5.5 KB) - added by mdawaffe 8 years ago.
Non-informative yet more contextual confirmation
2734b.diff (32.0 KB) - added by mdawaffe 8 years ago.
verbs and nouns and names, oh my!

Download all attachments as: .zip

Change History (15)

mdawaffe8 years ago

Non-informative yet more contextual confirmation

comment:1 ryan8 years ago

The only thing I can think of is that plugins that attach to the header or footer and do things such as add fields to forms might trigger since everything seems to be normal from their perspective. Maybe we should disable certain action and filter hooks when processing a nonce verification.

I'll go ahead and commit, and we can take it from there. Right now the dialog is so uninformative that people will probably just push Yes out of annoyance. This change should help. Maybe we could do a little something with the action name since it contains some information about what is being nonced. "delete-post1234"

comment:3 mdawaffe8 years ago

I'd thought of that, but it didn't seem worth it.

But I thought about it again and here it is.

2734b.diff

  1. Change nonce actions to 'verb-noun_ID': 'delete-post_1234', 'switch-theme_classic'. This was no easier to parse than 'delete_post1234' but much easier than 'switch-themeclassic'.
  2. If check_admin_referer() fails, an array of known verbs and an array of known nouns (and what function to call on ID, if any) is built. Both arrays try to be sensitive to i18n. If we have a known verb and noun pair, the AYS is made more specific: 'Are you sure you want to delete this post: "WP rocks"?', 'Are you sure you want to switch to this theme?'.

mdawaffe8 years ago

verbs and nouns and names, oh my!

comment:4 mdawaffe8 years ago

If we do end up changing the nonce action names, we'll have to backport. We don't want them to change between releases.

comment:5 ryan8 years ago

[3804] and [3805] for renamed action strings. check_admin_referer() changes not in yet.

comment:6 ryan8 years ago

Ideally, verb noun pairs would map to a sentence that has the verb and noun hardcoded.

__('Are you sure you want to delete category %s')

The nouns and verbs need the context of the entire sentence for proper translation. Also, we should move the nonce decoder and AYS dialog code to a separate function so that plugin writers don't have to reproduce all of that when providing a custom check_admin_referer(). I have to cut-and-paste all of that stuff into the secure-admin plugin even though I just want to add one line to the top of the function.

comment:7 ryan8 years ago

[3934] adds wp_nonce_ays() and wp_explain_nonce(). They're in functions.php for now, but could go in pluggable.php or in admin-functions.php since there is proboably no need of them outside of the admin. Might be nice to have pluggable functions that are only loaded for wp-admin so that we don't have to move admin-only functions into wp-includes to make them pluggable.

comment:8 ryan8 years ago

BTW, not all nonce actions are accounted for in the translation table. Anyone want to finish them off?

comment:9 ryan8 years ago

[3935] adds a few more.

comment:10 ryan8 years ago

[3936] for 2.0.

comment:11 ryan8 years ago

  • Milestone changed from 2.1 to 2.0.4
  • Version changed from 2.1 to 2.0.3

comment:12 ryan8 years ago

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

comment:13 anonymous7 years ago

  • Milestone 2.0.4 deleted

Milestone 2.0.4 deleted

Note: See TracTickets for help on using tickets.