• Resolved KZeni

    (@kzeni)


    As it is now, the notice shown suggesting people check out the HTML Validation plugin (https://www.ads-software.com/plugins/html-validation/) is dismissible, but that doesn’t have any code seeing if it’s been dismissed (per https://plugins.trac.www.ads-software.com/browser/wp-ada-compliance-check-basic/trunk/wp-ada-compliance-basic.php#L357 [it just sees if the plugin is installed or not & doesn’t see if the notice has been previously dismissed]).

    As such, it keeps showing this notice even when it’s previously been dismissed. Having this be a persistent notice is really something that can rub people the wrong way & have them look to go elsewhere to avoid having something like that always shown (for those that don’t want or otherwise have an alternative to the HTML Validation plugin.)

    ?Proposed fix for this notice:

    1. Make it so the dismissal triggers an AJAX call via a script that’s included alongside the notice which then has update_option save the dismissal to the site’s settings (using transients isn’t reliable for this & can still have it show more often than users want/expect so using options is the better way to go.)
    2. The dismissal option being saved could be a timestamp so determining if the notice is shown can show if that option isn’t set yet and/or the timestamp in the option is beyond a certain duration (ex. a year or so.) Alternatively, it could have the plugin version as what’s saved as the option so it shows whenever a different version of the plugin is being used from when it was last dismissed (probably more relevant than being time-based.) Then again, it could be a permanent dismissal of that message by using a simple boolean setting for it being dismissed.
    3. This notice could then still be shown 100% of the time on the plugin’s settings page for those looking to check/update things regarding the accessibility setup. I think that would be entirely reasonable & potentially helpful.
Viewing 13 replies - 1 through 13 (of 13 total)
  • Plugin Author seshelby

    (@seshelby)

    Hello, thank you for taking the time to suggest updates to the plugin. After completing a code review and testing on one of our websites I have found that the feature you are requesting already exists. The notice is dismissable for 30 days. If this is not what you are experiencing please review your javascript console to see if there are any related errors that may be contributing to this issue. You might also try disabling plugins to locate a plugin conflict. Please let me know if I can assist further.

    Thread Starter KZeni

    (@kzeni)

    Ah, the code indenting threw me off & I missed that part above the code I looked at.

    That said, it’s still showing on my site even after dismissing.

    What’s interesting is that https://www.website.com/wp-admin/index.php is showing the notice even though the database has eae_dismissed_notice-htmlvalidation-30 set to 1 for my user with it still being shown.

    Dismissing the notice does send an AJAX request

    
    MIME Type: application/x-www-form-urlencoded; charset=UTF-8
    notice: notice-htmlvalidation-30
    action: eae_dismiss_notice
    

    with it returning a 200 OK response.

    I have W3 Total Cache being used with Redis for database & object caching (among other optimizations) on PHP 7.2.x (currently) so I’m wondering if its related to that.

    However, I’m curious where eae_dismiss_notice is coming from as that’s being added into the dismissal info in the database & might be conflicting with it checking for a match.

    Thread Starter KZeni

    (@kzeni)

    Okay, it turns out that deactivating Email Address Encoder (https://www.ads-software.com/plugins/email-address-encoder/) has it working.

    Having EAE disabled also changed the AJAX request when dismissing this notice to contain

    
    MIME Type: application/x-www-form-urlencoded; charset=UTF-8
    action: dismiss_admin_notice
    option_name: notice-htmlvalidation
    dismissible_length: 30
    nonce: 540cb2b7bb
    

    So it seems EAE is somehow hijacking/conflicting with this setup.

    That said, EAE is a very useful & widely used plugin so addressing this conflict is probably worthwhile.

    Thread Starter KZeni

    (@kzeni)

    It largely seems due to EAE having:

    
    wp_enqueue_script(
        'dismissible-notices',
        plugins_url( 'dismiss-notice.js', __FILE__ ),
        array( 'jquery' )
    );
    

    while this plugin is using:

    
    wp_enqueue_script(
    	'dismissible-notices',
    	plugins_url( 'dismiss-notice.js', __FILE__ ),
    	array( 'jquery', 'common' ),
    	false,
    	true
    );
    

    The fact these are enqueueing their dismissal scripts with the same name has it so only one is loaded (with EAE then having higher priority so the dismissal script for this plugin isn’t loaded at all.)

    While changing the name of the script being loaded to something like wp_ada_compliance_basic_dismissible-notices has both scripts be loaded, it appears EAE is still hijacking the dismissal AJAX request & still has the notice shown after dismissal. So something more needs to be done to truly fix the conflict.

    Thread Starter KZeni

    (@kzeni)

    I’ve posted this at https://www.ads-software.com/support/topic/plugin-conflict-notice-dismissal-hijacking-notice-dismissal-of-other-plugins/ just in case there’s something to be done on EAE’s end of things.

    That said, this really is using naming & some selectors that are very generic & open to having conflicts (potentially with other plugins now & in the future) that should ideally be addressed either way. So there’s likely something to be updated for both plugins involved in this conflict.

    Is there a way to mark this as not resolved? It seems that was done even though the issue is very real & needs things updated, and it just needed some additional investigation to identify the cause of the problem.

    Plugin Author seshelby

    (@seshelby)

    Thanks for the additional information. I will get the generic naming issue taken care of. Hopefully the other plugin owner will address the hijacking issue as that isn’t something that I can resolve on my end.

    This problem is still occurring in v3.0.6 (also saw it in v3.0.5) along with the “WP ADA Compliance Basic is limited to 15 posts or pages, …” msg. not being dismissed as well.

    I am not using the Email Address Encoder plugin.

    Plugin Author seshelby

    (@seshelby)

    We can’t reproduce this issue. It is most likely another plugin breaking the dismissal option. You should be able to verify this by deactivating all your plugins accept WP ADA Compliance Basic and attempting to dismiss the message.

    I will have to disagree with you for 2 reasons.

    1. I disabled all plugins except for WP ADA Compliance Check Basic
    2. You’ve (presumably) taken care of any generic naming issues by changing to using:

    wp_enqueue_script(
    ‘wp-ada-compliance-basic-dismissible-notices’,
    plugins_url( ‘dismiss-notice.js’, FILE ),
    array( ‘jquery’, ‘common’ ),
    false,
    true
    );

    The “HTML code validation is an important…” msg. even shows on the Dashboard Home page after dismissing.

    Please confirm you tested with basic (free) version.

    Plugin Author seshelby

    (@seshelby)

    Thank you for clarrifing which notice had the issue. I was testing with a different notice. It appears the issue may be caused by using the same notice in multiple plugins. Thank you for your persistenace, We will get the issue resolved.

    Thread Starter KZeni

    (@kzeni)

    Yeah, this still isn’t fully resolved.

    I know if one thing that other plugins have ran into that caused issues which this plugin looks to also be running into (one example being https://www.ads-software.com/support/topic/thank-you-for-installing-menu-icons-any-way-to-hide-this-message-definitively/ among others).

    The key thing I noticed was https://plugins.trac.www.ads-software.com/browser/wp-ada-compliance-check-basic/trunk/res/vendor/persist-admin-notices-dismissal/persist-admin-notices-dismissal.php only wants to use transients via set_site_transient() and get_site_transient(). The problem here is that transients, by definition, aren’t persistent/permanent. They can set the duration for the maximum amount of time it’s valid, but they can be/are cleared out before that duration is met given certain actions & situations (purging of site caches, installing plugins, performing core/plugin updates, etc. per the site then wanting the latest version of things & therefore purging transients [again, with transients systematically being open to expire before their maximum expiration duration]) making the notice re-appear sooner than one wants/expects.

    The transient being cleared before it’s otherwise allowed to expire (again, something that can be triggered by any number of things) is something that’s easily resolved by using update_site_option() and get_site_option() instead (respectively).

    I was curious why that code snippet called itself “Persist Admin Notices Dismissal” while it then used something that isn’t as persistent as the alternative method (transient-based vs option-based, respectively.) I then checked it out and saw https://github.com/w3guy/persist-admin-notices-dismissal/blob/master/persist-admin-notices-dismissal.php has been seriously improved & bug-fixed in recent years to do just that… it now uses update_site_option() and get_site_option() (per the update they made back in 2018 according to https://github.com/w3guy/persist-admin-notices-dismissal/commit/b17c23d7e135eba9898a97f4fddb9d97eedc842a with their version 1.4.0 update.)

    – What looks to be needed now –

    This is hopefully a quick fix of updating this vendor component to no longer be using their pre-2018 (pre-1.4.0 version) code like this plugin is now and make it up-to-date by replacing it with what’s at https://github.com/w3guy/persist-admin-notices-dismissal (possibly needing the JS & other assets updated alongside the main PHP code for this) while then making sure it’s still enqueued given a unique name per the recent patch that was discussed & applied in this thread (avoid conflicts with other plugins while also now using options instead of transients to resolve this remaining issue.)

    Thread Starter KZeni

    (@kzeni)

    *Just in case my more lengthy comment gets stuck in moderation queue (had a bunch of links for references & things.) Ignore this comment if my comment from earlier today was approved and simply see that instead for the full details (though this comment could still be helpful as a summary.)

    The issue I’m seeing is that this is still using transients instead of options for storing the dismissal status which then is far from reliably persistent (they’re transient, after all.)

    The Persist Admin Notices Dismissal code in the vendor folder is from years ago whereas they fixed things back in 2018 with their version 1.4.0 to use options instead of transients.

    As such, this plugin just needs to update its Persist Admin Notices Dismissal code to be up-to-date (their GitHub shows 1.4.4 is the latest from 2021 [instead of the pre-2018/pre-1.4.0 version it has now]) so it then uses the more reliable method of storing notice dismissal status. Keep in mind it still needs to be enqueued with a unique name to avoid plugin conflicts as this thread previously discussed & got resolved, but at least they appear to have done the leg work of updating that one vendor component to handle this better for you (again, uses options instead of transients.)

    Plugin Author seshelby

    (@seshelby)

    Thank you for taking the time to research this and for providing the detailed suggestion to mitigate the issue. We will update the Persist Admin Notices Dismissal plugin and ensure that the issue is resolved.

Viewing 13 replies - 1 through 13 (of 13 total)
  • The topic ‘Honor the HTML Validation suggested plugin notice’s dismissal’ is closed to new replies.