WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#30859 closed defect (bug) (fixed)

Capability limit on custom taxonomy does not allow editors to preview

Reported by: MikeNGarrett Owned by: boonebgorges
Milestone: 4.2 Priority: normal
Severity: normal Version: 4.2
Component: Taxonomy Keywords: has-patch
Focuses: javascript, administration Cc:

Description

This is a super-niche bug, but I believe my potential solution makes post.js more hardy.

I'm registering a custom taxonomy with capabilities to limit it in the following way:

	'capabilities'      => array(
		'manage_terms'  => 'manage_options',
		'edit_terms'    => 'manage_options',
		'delete_terms'  => 'manage_options',
		'assign_terms'  => 'manage_options'
	),

This allows admins to manage the terms and all other users to view any terms that have been set.

The full register taxonomy:

register_taxonomy( 'migrated_category', array( 'post' ), array(
	'hierarchical'               => false,
	'public'                     => false,
	'show_ui'                    => true,
	'show_admin_column'          => false,
	'show_in_nav_menus'          => true,
	'show_tagcloud'              => false,
	'capabilities'      => array(
		'manage_terms'  => 'manage_options',
		'edit_terms'    => 'manage_options',
		'delete_terms'  => 'manage_options',
		'assign_terms'  => 'manage_options'
	),
	'labels'            => array(
		'name'                       => 'Migrated Categories',
		'singular_name'              => 'Migrated Category',
		'menu_name'                  => 'Migrated Categories',
	),
) );

As an editor, when you publish or save a new post, the terms for migrated categories are not set. This works fine except when attempting to preview this published or saved content. The following error is thrown in post.js

line 30: Uncaught TypeError: Cannot read property 'replace' of undefined

tagBox.flushTags() is not expecting both the existing tags and new tags to be undefined. There isn't a check here, so it errors out as tagBox.clean() gets called, expecting tags to be passed to it.

I'm attaching a patch that checks for typeof undefined on both tagBox.flushTags() and tagBox.clean(). It may be redundant since it seems like the flush and parse are the only things that call clean, but I'd rather be safe.

To note, removing capabilities from the taxonomy works as does having a term already set. Oddly enough, for admins in the same situation this is not a problem at all.

Attachments (3)

30859-post-js.diff (687 bytes) - added by MikeNGarrett 4 years ago.
post.js adding checks for typeof undefined
30859.patch (466 bytes) - added by A5hleyRich 4 years ago.
Stop undefined appending to tags list.
30859-tags-box.patch (746 bytes) - added by MikeNGarrett 4 years ago.
4.2 patch

Download all attachments as: .zip

Change History (13)

@MikeNGarrett
4 years ago

post.js adding checks for typeof undefined

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


4 years ago

#2 @SergeyBiryukov
4 years ago

  • Keywords has-patch added
  • Milestone changed from Awaiting Review to 4.2

#3 @DrewAPicture
4 years ago

Hi Mike, welcome to Trac!

I can't seem to reproduce the TypeError you're getting. I did notice that with the patch applied the undefined keyword did start to appear in the taxonomy meta box which was ... interesting.

30859-post-js.diff still applies.

#4 @valendesigns
4 years ago

  • Keywords needs-refresh added; has-patch removed

The patch no longer applies and needs a refresh. The code has been moved into the new tags-box.js file. As well, I can't reproduce the error and only see an empty meta box for Migrated Categories, even though I created a few taxonomies as an admin. Maybe I'm missing something in getting this setup. In any case a new patch is needed. Cheers!

#5 @A5hleyRich
4 years ago

I can't reproduce the TypeError either. However, as Drew has said when you hit 'Preview Changes' as an Editor, the undefined keyword is added as a tag. This happens on trunk without the patch applied.

@A5hleyRich
4 years ago

Stop undefined appending to tags list.

#6 @A5hleyRich
4 years ago

  • Keywords has-patch added; needs-refresh removed

@MikeNGarrett
4 years ago

4.2 patch

#7 @MikeNGarrett
4 years ago

Same setup. Latest from trunk (4.2). I am getting the same result, but a different error in js:

undefined is not an object (evaluating 'tags.replace')

To note: this is the editor role previewing a post they've published or saved with a custom taxonomy.

Attached is a new patch to tags-box.js.

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


4 years ago

#9 @boonebgorges
4 years ago

Thanks, MikeNGarrett. I've reproduced the issue. I think you're right that the double check is redundant, so I'm going to trace it up to where the first undefined variable is found in flushTags(), and put the check there. (The call to clean() in parseTags() is never reached for a disabled taxonomy metabox - it's inside of an each() loop over an empty array.)

#10 @boonebgorges
4 years ago

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

In 31895:

When saving post, ensure that non-hierarchical taxonomy input is defined before attempting to parse it.

Taxonomy metaboxes that are disabled for the current user are included in the
post.php markup, but do not contain the 'newtag' field, and so should be
skipped when looping through the metaboxes to avoid invoking String methods
on a variable of type undefined.

Props MikeNGarrett, A5hleyRich.
Fixes #30859.

Note: See TracTickets for help on using tickets.