In #2882175: Accessibility: Close popover when ESC is pressed, code was added which assumes that jQuery UI is loaded. This results in the following error: "Uncaught Error: Syntax error, unrecognized expression: unsupported pseudo: focusable" if the sub theme doesn't use jQuery UI, which is commonly the case for anonymous visitors. Patch to follow.

CommentFileSizeAuthor
#3 3013236-3.patch1.49 KBmarkhalliwell
#2 bootstrap--check-for-jquery-ui-3013236-2.patch1.68 KBloopduplicate

Comments

loopduplicate created an issue. See original summary.

loopduplicate’s picture

loopduplicate commented
Assigned: loopduplicate » Unassigned
Status: Active » Needs review
StatusFileSize
new bootstrap--check-for-jquery-ui-3013236-2.patch1.68 KB

Here's a patch which pulls the anonymous behavior function out into a named function and later checks to see if jquery.ui is available before attempting to use :focusable.

markhalliwell’s picture

markhalliwell
Primary language English
commented
Title: Check for jQuery UI before using :focusable » Uncaught Error: Syntax error, unrecognized expression: unsupported pseudo: focusable
Status: Needs review » Fixed
StatusFileSize
new 3013236-3.patch1.49 KB

#2 isn't really much of a solution since it would work some of the time and just confuse people with unknown "random" behavior.

We can just get away with using :visible instead. It was merely meant as a way to activate jQuery's "global focus" magic.

  • markcarver committed 26ac6c3 on 8.x-3.x 
    Issue #3013236 by markcarver, loopduplicate: Uncaught Error: Syntax...
loopduplicate’s picture

loopduplicate commented

Thanks for the commit. I see how my approach was wrong now, whoops. I figured my approach wouldn't be the one that got accepted but I needed a bandaid last night :)

markhalliwell’s picture

markhalliwell
Primary language English
commented

Not sure about the 'unknown "random" behavior' you are referring to

jQuery UI can still be loaded on some pages were certain widgets like drag/resize that are commonly used throughout Drupal sites.

This means that some users will be able to close by losing focus on another element and some won't and the popover would still be visible.

Thus, this can lead to what some users would classify as "random" because they wouldn't know why it would close on some pages and not others.

loopduplicate’s picture

loopduplicate commented

Yeah, sorry Mark. I was tired and read the code wrong the first time. I didn't realize that you were using the second parameter as delegation. For some reason, my brain was thinking you were using multiple selectors in the first argument. Sigh. Guess that's what I get for jumping back and forth from front end and back end so much and for taking a couple of months off, pretty much :) Cheers.

markhalliwell’s picture

markhalliwell
Primary language English
commented

Heh, no worries. I'm the one who introduced this bug, so... we all make mistakes :D

markhalliwell’s picture

markhalliwell
Primary language English
commented

FTR/explanation: the reason #2 appeared to work for you last night is because it essentially removed the entire focus code from play.

  1. +++ b/js/popover.js
@@ -59,13 +66,7 @@ var Drupal = Drupal || {};
+        .on('focus.bs.popover', focusBehavior)

    Doesn't actually do anything because there really isn't a global "focus" event. That's why the second delegate parameter is needed so it triggers jQuery's magic (it's not a widely known or documented feature). 

  2. +++ b/js/popover.js
@@ -81,6 +82,10 @@ var Drupal = Drupal || {};
+        $document.on(':focusable', focusBehavior);

    Doesn't actually do anything because :focusable isn't a valid event to bind, or rather... nothing actually triggers an event with that name.

markhalliwell’s picture

markhalliwell
Primary language English
commented

FYI, I've been using Bootstrap's popover demo code in a random node to test all this:

https://monosnap.com/file/BxlEyoMrAVmax1hU9DqLFx6jomcIf5

edit: updated screencast URL, which includes fix from #3013402: Popover data-auto-close attribute not respected

ressa’s picture

ressa commented

I just noticed this warning:

Syntax error, unrecognized expression: unsupported pseudo: focusable

Thanks for fixing it! Is there a temporary solution until the patch is included in the official release, or is a fresh release close?

markhalliwell’s picture

markhalliwell
Primary language English
commented

Just use the patch until another release is made. I may do another one later this month or perhaps next month... just have to see how it falls.

ressa’s picture

ressa commented

Sure. For others, here is an article on How to apply patches in Drupal 8 with Composer, and the bit for this patch: 

        "patches": {
            "drupal/bootstrap": {
                "Fixes Uncaught Error ... unsupported pseudo: focusable": "https://www.drupal.org/files/issues/2018-11-13/3013236-3.patch"
            }
        }
loopduplicate’s picture

loopduplicate commented

You can also get the dev version, which already includes the patch.

ressa’s picture

ressa commented

I know @loopduplicate, and thanks for the tip, but I need to use the latest stable release in composer.json.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.