WordPress.org

Make WordPress Core

Opened 2 years ago

Last modified 8 months ago

#36263 assigned defect (bug)

When building a shortcode with `wp.shortcode.string()` the `attrs` property is finnicky.

Reported by: georgestephanis Owned by: yale01
Milestone: Awaiting Review Priority: low
Severity: minor Version: 3.5
Component: Shortcodes Keywords: dev-feedback 2nd-opinion good-first-bug has-patch has-unit-tests
Focuses: javascript Cc:

Description

If you pass it in as:

wp.shortcode.string({
	tag   : 'short',
	type  : 'single',
	attrs : {
		named   : { foo : 'bar' },
		numeric : [ 'abc123' ]
	}
});

it works fine -- resulting in [short abc123 foo="bar"], but if you flip the order so that numeric comes before name, like so:

wp.shortcode.string({
	tag   : 'short',
	type  : 'single',
	attrs : {
		numeric : [ 'abc123' ],
		named   : { foo : 'bar' }
	}
});

the output is [short numeric="abc123" named="[object Object]"]

This is because we're doing an overly basic array comparison of the keys in which order matters --

https://core.trac.wordpress.org/browser/trunk/src/wp-includes/js/shortcode.js?rev=34933#L218

// Identify a correctly formatted `attrs` object.
} else if ( _.isEqual( _.keys( attrs ), [ 'named', 'numeric' ] ) ) {
	this.attrs = attrs;

I wonder if we could do something as simple as using _.difference instead --

} else if ( _.difference( _.keys( attrs ), [ 'named', 'numeric' ] ).length === 0 ) {
	this.attrs = _.defaults( attrs, { named : {}, numeric : [] } );

or something?

Open to suggestions.

Attachments (2)

36263.diff (628 bytes) - added by yale01 16 months ago.
improved array keys comparison
36263.2.diff (1.7 KB) - added by adamsilverstein 16 months ago.

Download all attachments as: .zip

Change History (14)

#1 @georgestephanis
2 years ago

  • Keywords good-first-bug added

#2 @zsusag
2 years ago

@georgestephanis

In theory that seems to work; however, I'm wondering how one would be able to use wp.shortcode.string(). I'm relatively new here to the forums and to WordPress core but if I am correct most users implement shortcode via PHP instead of JavaScript. Am I mistaken in this? I'm trying to replicate your bug.

#4 @georgestephanis
2 years ago

But perhaps more to your question -- when you're inserting media from the media modal in the editor screen into a post body, yes, it fires an ajax request to get the shortcode generated by php. Because legacy reasons. But if you're using Editor Views and editing them in JS, that all happens client-side, purely in JS.

#5 @zsusag
2 years ago

@georgestephanis Ahh, I see. Thank you for the examples and explanation.

With the greater insight that does seem to be a better and more "friendly" way of handling things. I haven't found time to formally set up a test for your code but it does look like that would be a more intuitive way of going about things.

#6 @obenland
2 years ago

  • Version changed from trunk to 3.5

#7 @mircoraffinetti
23 months ago

@georgestephanis: just a short feedback. We tested your suggested code and it works fine.

#8 @DrewAPicture
21 months ago

@georgestephanis Any follow up here?

@yale01
16 months ago

improved array keys comparison

#9 @yale01
16 months ago

patch added, from code provided by @georgestephanis and tested againt the case he provided

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

#10 @yale01
16 months ago

  • Keywords has-patch added

#11 @adamsilverstein
16 months ago

  • Keywords has-unit-tests added

36263.2.diff adds a quint test verifying the fix

#12 @DrewAPicture
8 months ago

  • Owner set to yale01
  • Status changed from new to assigned

Assigning ownership to mark the good-first-bug as "claimed".

Note: See TracTickets for help on using tickets.