roots / acorn

Laravel components for WordPress plugins and themes
https://roots.io/acorn/
MIT License
817 stars 94 forks source link

Gravity forms merge tags error in form notifications #198

Closed Twansparant closed 2 years ago

Twansparant commented 2 years ago

Terms

Description

What's wrong?

I can't seem to insert any Gravity forms merge tags on the forms notification edit screens. It it working on the forms confirmation edit screens.

What have you tried?

What insights have you gained?

Possible solutions

Temporary workarounds

Steps To Reproduce

  1. Install the Gravity forms plugin
  2. Create a form and go to it's notifications page
  3. Try to insert a merge tag in the from email address field

Expected Behavior

I expect a dropdown list with the all form fields merge tags to appear.

Actual Behavior

Nothing happens, I get an error in my browser console.

Relevant Log Output

Uncaught TypeError: Cannot read properties of null (reading 'value')
    at MaybeAddSaveLinkMergeTag (admin.php:1227:54)
    at admin.php:78:22
    at Array.forEach (<anonymous>)
    at Object.doHook (admin.php:70:11)
    at Object.applyFilters (admin.php:38:17)
    at gfMergeTagsObj.getMergeTags (form_admin.js:1322:21)
    at gfMergeTagsObj.getMergeTagListItems (form_admin.js:1463:24)
    at HTMLAnchorElement.<anonymous> (form_admin.js:952:35)
    at HTMLAnchorElement.dispatch (jquery.js:5430:27)
    at HTMLAnchorElement.elemData.handle (jquery.js:5234:28)

Versions

2.0.0

retlehs commented 2 years ago

@Twansparant this doesn't look like a bug report, this looks like a support request. if you can re-produce this on a wordpress install with acorn and no other modifications then let us know, otherwise please open a topic on roots discourse

Twansparant commented 2 years ago

I know it doesn't look like one, but I can re-produce this with a bare installation of:

With no modifications whatsoever.

So I boiled it down to the usage of Acorn. When I switch from the Sage starter theme to the Twenty Twenty-Two theme, the error is gone.

retlehs commented 2 years ago

that doesn't yet make it an acorn bug, it could be a sage bug? acorn shouldn't be doing anything that would be causing a front-end javascript error? let us know if that's not the case tho

edwinsiebel commented 2 years ago

Seems like the same issue I encountered (and reported to Gravityforms):

The error occured because there was no element with id="event" in the admin.

The attributes for the field "gform_setting_event" returned an empty array, instead of at least an "id"-key and "event"-value. Digging deeper why this happens, I found in the "class-base.php" in "includes/settings/fields/" directory, the method "get_attributes", and almost on the of this method, it is preparing the attributes as strings.

The $atts array has a value of:

[ 'id' => 'event' ];

It iterates over and fills the $return value. What happened in my situation, was the 'is_callable($value)' [ thus eventually calling 'is_callable('event')' ] returned true. This resulted in not including this key and value, and thus failing above.

The reason 'event' is callable, is because we include the Laravel Framework, which includes a global function called.... 'event': https://github.com/laravel/framework/blob/8.x/src/Illuminate/Foundation/helpers.php#L439.

So, I can imagine not many encounter above situation. However, I think the way GF is handling the attibutes is not correct. A global function defined anywhere in code, could break the logic in the get_attributes() method.

Twansparant commented 2 years ago

True, but when I disable the Register The Bootloader & Register Sage Theme Files calls in Sage, the error is also gone?

\Roots\bootloader();
edwinsiebel commented 2 years ago

I'm not using roots, so not familiar with that.

However, if Laravel is loaded (with the helpers file), the error occured at my end.

Maybe the register bootloader loads this specific file?

QWp6t commented 2 years ago

Thanks for the explanation @edwinsiebel.

Yes, Acorn loads Laravel helpers.

@Twansparant this is a bug in Gravity Forms.

A workaround on your end could be to create a global event() function that loads before Acorn loads; the function would be in charge of handling calls from GF and Acorn. Out of the box, Acorn doesn't use that function, so you might be able to get away with assuming that only GF will call the function.

Feel free to make a post about this on Roots Discourse: https://discourse.roots.io/

Twansparant commented 2 years ago

@edwinsiebel Thanks for your reply! That makes sense. I also reported this to gravity forms, but since it was not occuring on the default theme, I thought later that it must be caused on my end and not theirs.

Twansparant commented 2 years ago

@QWp6t Check, will try that and I'll report back! There must be quit a few people though using Laravel & Gravityforms? Thanks for the tips!

Twansparant commented 2 years ago

A workaround on your end could be to create a global event() function that loads before Acorn loads; the function would be in charge of handling calls from GF and Acorn.

The problem is, that the function shouldn't be callable in the first place, so even if I override the event function, it will still fail in returning the value.

Twansparant commented 2 years ago

So this is not related to acorn indeed, since it also happens with any of the default themes by just adding a function called event to to functions.php:

function event() {
    return true;
}
davideprevosto commented 2 years ago

@Twansparant I have noticed the identical issue. Have you maybe found any fix for this? Thank you

Twansparant commented 2 years ago

Nope, no fix unfortunately... I filed a bug report to GravityForms but they don't think it's a bug on their end, so I could only submit it as a product suggestion

Twansparant commented 2 years ago

I'm still having this issue, and I still don't know how to fix it... I tried something like this in my functions.php that basically adds an event function that shouldn't only be callable once by Acorn and then never again:

global $event_called;
$event_called = false;

if (!$event_called) {
    function event() {
        global $event_called;
        $event_called = true;
    }
}

try {
    \Roots\bootloader();
} catch (Throwable $e) {
    wp_die(
        __('You need to install Acorn to use this theme.', 'sage'),
        '',
        [
            'link_url' => 'https://docs.roots.io/acorn/2.x/installation/',
            'link_text' => __('Acorn Docs: Installation', 'sage'),
        ]
    );
}

And in my app/Providers/ThemeServiceProvider.php:

class ThemeServiceProvider extends ServiceProvider {
    public function boot() {
        event();
    }
}

When I log function_exists('event') in Acorn's helpers file it returns true, so that works.

When I log is_callable('event') in the GravityForms get_attributes function in includes/settings/fields/class-base.php on L328 it also returns true, so that doesn't work.

So basically how can you make a function uncallable, is that even possible?

QWp6t commented 2 years ago

I would encourage you to open a discussion about this on our forums. https://discourse.roots.io

But in short you can try declaring the function in mu-plugins or even before WordPress is loaded, such as part of wp-config.

tombroucke commented 1 year ago

I ran across the same issue today. I found an easy workaround, by just redefining the MaybeAddSaveLinkMergeTag function with a better dom query (which doesn't rely on the id attribute) later on the page.

add_action('admin_footer', function () {
    $isGravityFormsEditPage = isset($_GET['page']) && 'gf_edit_forms' === $_GET['page'];
    if (!$isGravityFormsEditPage) {
        return;
    }
    ?>
    <script type="text/javascript">
    function MaybeAddSaveLinkMergeTag( mergeTags, elementId, hideAllFields, excludeFieldTypes, isPrepop, option ) {
        const eventSelectEl = document.querySelector('select[name="_gform_setting_event"]');
        if(!eventSelectEl) {
            return mergeTags;
        }
        var event = eventSelectEl.value;
        if ( event === 'form_saved' || event === 'form_save_email_requested' ) {
            mergeTags['other'].tags.push( {
                tag:  '{save_link}',
                label: <?php echo json_encode(esc_html__('Save & Continue Link', 'gravityforms')); ?>
            } );
            mergeTags['other'].tags.push( {
                tag:   '{save_token}',
                label: <?php echo json_encode(esc_html__('Save & Continue Token', 'gravityforms')); ?>
            } );
        }
        return mergeTags;
    }
    </script>
    <?php
});
Twansparant commented 1 year ago

Thanks @tombroucke, gonna try that first thing in the morning!

Twansparant commented 1 year ago

Yes your fix works like a charm @tombroucke Many thanks!

christianmagill commented 1 year ago

@tombroucke , Is this still the best solution we have?

mgussekloo commented 11 months ago

Another solution; adding the missing attribute so Gravity Forms javascript function can work as intended.

<?php

if ( ! class_exists( 'GFForms' ) ) {
    die();
}

add_action('admin_footer', function () {
    $isGravityFormsEditPage = isset($_GET['page']) && 'gf_edit_forms' === $_GET['page'];
    if (!$isGravityFormsEditPage) {
        return;
    }
    ?>
    <script type="text/javascript">document.querySelector('select[name="_gform_setting_event"]').setAttribute('id', 'event');</script>
    <?php
}, 100);