wp-shortcake / shortcake

Shortcake makes using WordPress shortcodes a piece of cake.
GNU General Public License v2.0
664 stars 142 forks source link

Shortcode attributes not passed through to AJAX functions #520

Closed drewbaker closed 8 years ago

drewbaker commented 9 years ago

If I have a short code like this:

[product-image-block left-attachment-id="166" product-attachment-id="408" product-url="example.com" product-brand="Kinfolk" product-name="Spotted Shirt" product-price="$99"]
     <h2>December 50% Off</h2>
         Free shipping on everything!
     <a href="http://example.com">View sale item</a>
[/product-image-block]

It fires off this AJAX request: screen shot 2015-10-22 at 1 50 09 pm

Notice that the request is missing all the parameters, so the HTML that returns is wrong obviously too.

Does anyone else have this problem? Is there anything that could conflict with that?

khromov commented 9 years ago

The shortcode content is sent, and the parameters are parsed on the backend. (Just like WP Core does it.)

drewbaker commented 9 years ago

@khromov It doesn't look like the attributes get sent though. Like left-attachment-id="166" product-attachment-id="408" isn't included in the POST request?

It might be due to bug #495 though.

goldenapples commented 9 years ago

It might be due to bug #495 though.

Yup, that seems to be the issue here. Can you reproduce with any non-hyphenated attributes?

drewbaker commented 9 years ago

Actually, no it doesn't work even without hyphens.

This short code [product id="1233"]Test[/product] produces this POST request:

screen shot 2015-10-22 at 2 19 19 pm

drewbaker commented 9 years ago

In case it's relevant, the site also has WooCommerce installed on it, but otherwise no other plugins.

goldenapples commented 9 years ago

hmm. That is strange. What version of Shortcake are you running? The current master has some slightly different behavior (it would be calling action=bulk_do_shortcode, rather than action=do_shortcode, for example.)

drewbaker commented 9 years ago

Version 0.5.0 from the WordPress.org plugin app store (or whatever they call that).

goldenapples commented 9 years ago

That clearly seems like a bug, but before we get too deep into tracking it down, can you try downloading the current master version from here and see if you still get the same issue?

drewbaker commented 9 years ago

Nope, still not passing attributes to AJAX:

screen shot 2015-10-22 at 3 25 33 pm

goldenapples commented 9 years ago

I can reproduce locally. When you have a registered shortcode which has unregistered attributes, those attribute values, which should be set in backup_attributes on the shortcode model, don't get passed through shortcodeModel.formatShortcode to the initial preview request:

screen shot 2015-10-23 at 11 52 27 am

It's a bug related to #425, which was missed at the time.

drewbaker commented 9 years ago

@goldenapples You say unregistered attributes, is there a way to register them? Did I miss something in my setup? I haven't seen anything about backup_attributes in the docs/dev demo.

The way I set "backups", is like this:

    $attr = shortcode_atts( array(
        'source'     => '',
        'attachment' => 0,
        'source'     => null,
    ), $attr, $shortcode_tag );

PS: I love this plugin. Should be in core, in my opinion.

goldenapples commented 9 years ago

is there a way to register them?

Maybe I'm misunderstanding your situation, but the way I was able to reproduce your issue was by using a shortcode which was registered with a callback function, but using attributes which were not registered in the shortcode_ui_register_for_shortcode() call which registered the UI for the shortcode.

backup_attributes is something completely internal to Shortcake - if a shortcode includes attribute values which Shortcake doesn't know about, it's supposed to save them to a backup attributes data structure when a user opens the edit modal, and then restore them to attributes when closing the modal to avoid data loss. The issue you're seeing is a case where our shortcode parser isn't doing that work on initial load of a shortcode preview.

drewbaker commented 9 years ago

This is my function that uses shortcode_ui_register_for_shortcode(). Seems like everything is correct, but I still don't get the attributes passed to to POST request. Am I missing something that would fix this? Or is it the bug?

    function product_image_shortcode_ui() {

        // Check if Shortcake exists
        if ( ! function_exists( 'shortcode_ui_register_for_shortcode' ) ) {
            return;
        }

        // Create our UI
        shortcode_ui_register_for_shortcode(
            'product-image-block',
            array(
                // Display label. String. Required.
                'label'             => 'Product with Image Block',
                'listItemImage'     => 'dashicons-cart',
                'post_type'         => array('page', 'post', 'lookbook', 'product'),
                'inner_content'     => array(
                    'label' => 'Text that will appear over left image on hover',
                ),              
                'attrs' => array(
                    array(
                        'label' => 'Left Landscape Image',
                        'attr'  => 'left-attachment-id',
                        'type'  => 'attachment',
                        'libraryType' => array('image'),
                        'addButton'   => 'Select Image',
                        'frameTitle'  => 'Select Image'                     
                    ),                  
                    array(
                        'label' => 'Product Portrait Image',
                        'attr'  => 'product-attachment-id',
                        'type'  => 'attachment',
                        'libraryType' => array('image'),
                        'addButton'   => 'Select Image',
                        'frameTitle'  => 'Select Image'                     
                    ),    
                    array(
                        'label'    => 'URL to link the product to',
                        'attr'     => 'product-url',
                        'type'     => 'url',
                        'meta' => array(
                            'placeholder' => 'http://www.example.com'
                        )                       
                    ),                                  
                    array(
                        'label' => 'Product Brand',
                        'attr'  => 'product-brand',
                        'type'  => 'text',
                        'meta' => array(
                            'placeholder' => 'Kinfolk'
                        )
                    ),
                    array(
                        'label' => 'Product Name',
                        'attr'  => 'product-name',
                        'type'  => 'text',
                        'meta' => array(
                            'placeholder' => 'Bomber jacket'
                        )
                    ),                  
                    array(
                        'label' => 'Product Price',
                        'attr'  => 'product-price',
                        'type'  => 'text',
                        'meta' => array(
                            'placeholder' => '$100.00'
                        )
                    )
                )
            )
        );    
    }    
    add_action( 'admin_init', 'product_image_shortcode_ui' );    
goldenapples commented 9 years ago

I've tested this situation in a bunch of different permutations, and I think there are two unrelated bugs here.

The issue you're describing above is a result of the javascript regex not supporting hyphenated attributes. (#495) Even when the attributes on the shortcode are registered properly, the WordPress core javascript attribute parser doesn't handle hyphenated attributes. This is an upstream bug which is being tracked at https://core.trac.wordpress.org/ticket/34191

The issue you found above with the test shortcode [product id="1233"]Test[/product] seems to be a new bug related to unregistered atttributes not being passed to the preview on first render. I'll work on tracking down what's happening with that bug.

danielbachhuber commented 8 years ago

seems to be a new bug related to unregistered atttributes not being passed to the preview on first render. I'll work on tracking down what's happening with that bug.

This is because shortcodeViewConstructor.initialize() uses getShortcodeModel() on an already-parsed shortcode: https://github.com/wp-shortcake/shortcake/blob/065d6ddc970a6c3c673e10336e452988ca6dfe3f/js/src/utils/shortcode-view-constructor.js#L28

getShortcodeModel() doesn't do anything about backing up attributes or inner content.

Later, when parseShortcodeString() is used, it does properly back up attributes and inner content.

I'm not sure why we have parseShortcodeString() when core seems to parse shortcodes just fine. I think we should make it a light wrapper for core's shortcode parsing, and then move the setting backup logic to getShortcodeModel

jesperbjerke commented 8 years ago

Sidenote: if you yank out parseShortcodeString() in favor for core's equivalent, latest issue in #501 would probably get solved also.

drewbaker commented 8 years ago

Stoked on the progress guys, really love this plugin and good to see the teams behind it!

danielbachhuber commented 8 years ago

The hyphenated attributes issue is tracked with #495, so this issue is good to :ship:

drewbaker commented 8 years ago

Guys I'm still seeing this with v0.6.1. Unless it's waiting on the hyphenated attributes issue to be fixed by WordPress.

danielbachhuber commented 8 years ago

Unless it's waiting on the hyphenated attributes issue to be fixed by WordPress.

Yes — that will be fixed in WP 4.3.2

R-Atik commented 1 year ago

I found same issue still now in 2023!