Problem/Motivation

We see a lot of javascript errors, thrown from the querySelectorAll call in active-link.js, when the URL contains characters that are not allowed by querySelector.

```
Uncaught DOMException: Failed to execute 'querySelectorAll' on 'Element': '[data-drupal-link-system-path="documenten"]:not([hreflang])[data-drupal-link-query='{"id":"848","scroll-to":"[data-document-id='848']"}'],[data-drupal-link-system-path="documenten"][hreflang="en"][data-drupal-link-query='{"id":"848","scroll-to":"[data-document-id='848']"}']' is not a valid selector.
```

Steps to reproduce

On a local Drupal 10.1 installation, navigate to this path:

/?scroll-to=%5Bdata-document-id%3D%27848%27%5D

Open the browser's dev tools, observe the errors.

Proposed resolution

Wrapping the selectors.join(',') call in CSS.escape() fixes the issue.

CommentFileSizeAuthor
#3 3396299-3.patch587 bytes_utsavsharma
#3 interdiff_2-3.txt430 bytes_utsavsharma
#2 active-link-css-escape.patch586 bytesroderickgadellaabsl

Issue fork drupal-3396299

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

roderickgadellaabsl created an issue. See original summary.

roderickgadellaabsl’s picture

roderickgadellaabsl commented
StatusFileSize
new active-link-css-escape.patch586 bytes

See patch (attached) for 10.1.x

_utsavsharma’s picture

_utsavsharma commented
Status: Active » Needs review
StatusFileSize
new interdiff_2-3.txt430 bytes
new 3396299-3.patch587 bytes

Fixed failures in #2.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Should contain test coverage.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Aadhar_Gupta made their first commit to this issue’s fork.

jrb’s picture

jrb
he/him
Primary language English
Location Raleigh-Durham Area, NC, USA
commented
Status: Needs work » Closed (duplicate)

I think this is a duplicate of #2902769: active-link.js throws JS error if query string parameter contains a single quote

Per that issue, it seems that the actual problem is the ' (single quote) in the queryString variable that causes this line of JavaScript (prior to what the patch here changes) to produce what will be an invalid selector:

      const querySelector = path.currentQuery
        ? `[data-drupal-link-query='${queryString}']`
        : ':not([data-drupal-link-query])';

In that issue, the solution is to escape the single quote. That fixes this issue as well. The use of CSS.escape in the patch of this issue, prevents the errors from being thrown, but I don't think it actually fixes the problem. The resulting selector still won't be valid-- it will just be escaped. I think the solution in that issue is the way to go.

jrb’s picture

jrb
he/him
Primary language English
Location Raleigh-Durham Area, NC, USA
commented
Related issues: +#2902769: active-link.js throws JS error if query string parameter contains a single quote
roderickgadellaabsl’s picture

roderickgadellaabsl commented

Hmm it's not just `queryString`, any query parameter could contain a single quote (or other characters that would cause active-link to throw)?

Since active-link pipes content through `querySelector()` that it does not control, I think it's safer to just `CSS.escape()` it all?