invalid selector regression in 5.3.2

[codylerum]

Prerequisites

• I have searched for duplicate or closed issues
• I have validated any HTML to avoid common problems
• I have read the contributing guidelines

Describe the issue

After updating to Bootstrap 5.3.2 we have errors where modals are failing to open due to the query selector failing as invalid

selector-engine.js:41 Uncaught DOMException: Failed to execute 'querySelector' on 'Element': '#j_id22:exampleModal' is not a valid selector.
    at Object.findOne (https://cdn.jsdelivr.net/npm/bootstrap@latest/dist/js/bootstrap.bundle.min.js:6:8558)
    at Object.getElementFromSelector (https://cdn.jsdelivr.net/npm/bootstrap@latest/dist/js/bootstrap.bundle.min.js:6:9299)
    at HTMLButtonElement.<anonymous> (https://cdn.jsdelivr.net/npm/bootstrap@latest/dist/js/bootstrap.bundle.min.js:6:55221)
    at HTMLDocument.n (https://cdn.jsdelivr.net/npm/bootstrap@latest/dist/js/bootstrap.bundle.min.js:6:4238)

The target is like data-bs-target="#j_id22:exampleModal". This is a common auto generated identifier from JSF where : is used as a seperator.

This code works in 5.3.1 but fails in 5.3.2

Reduced test cases

Working in 5.3.1: https://codepen.io/codylerum/details/OJrjvBx

Failing in 5.3.2: https://codepen.io/codylerum/pen/WNLEzKz

What operating system(s) are you seeing the problem on?

macOS

What browser(s) are you seeing the problem on?

Chrome

What version of Bootstrap are you using?

5.3.2

[julien-deramond]

Thanks for reporting this issue @codylerum.
Here's a first quick analysis. I don't have the complete history so I'll make some assumptions.

I tried your example in different Bootstrap versions:

• v5.1.3: didn't work
• v5.2.0: didn't work
• v5.2.1: didn't work
• v5.2.2: didn't work
• v5.2.3: didn't work
• v5.3.0: worked
• v5.3.1: worked
• v5.3.2: doesn't work anymore

So it worked from the following commit fcdfee9 which was a refactoring. The corresponding PR doesn't mention such new support.
The commit just before (ef4e2da) didn't work with this example.

In v5.3.2, we roll-backed a part of this refactoring commit which broke the fact to handle multiple IDs (see 9900cf3).

Maybe I'm wrong, but without much information, I'd say that it worked during this time (v5.3.0 and v5.3.1) by chance.

If this analysis is correct, we could try to make it work by modifying the getSelector and by adding this use case in our unit tests.

Thoughts @twbs/js-review?

(Putting it in v5.3.3 project for now to not forget it)

[codylerum]

@julien-deramond It looks like previously our code that generated this inserted a \ before the : due to how it worked before with jquery. When we upgraded to 5.3.1 this no longer worked so we removed the \

button: data-bs-target="#j_idt555\:uploadModal"

modal: id="j_idt555:uploadModal"

[XhmikosR]

We need a test for sure and we should see if we could do a workaround. Otherwise, we might need to revert #38989 and cut a new patch release soon.

/CC @GeoSot

[julien-deramond]

Some precisions here after a few other quick tests.

Considering the CodePen provided by OP, if we execute a simple document.querySelector, here's what happens:

As pointed out in #39189 (comment), some characters need to be escaped when we want to call querySelector.

#38989 moved the parseSelector() back to where it was to handle the multiple ids. However, removing the parseSelector() from the return operation doesn't escape the characters anymore in the case of an ID containing special characters, so the final call to findOne (which is basically a call to querySelector()) fails with #j_id22:exampleModal for instance.

But putting it back escapes , which breaks the handling of multiple IDs (#hello\,#world instead of #hello,#world).

We could probably do a workaround here if we are capable of knowing in which case we are and return either the selector or parseSelector(selector) in getSelector() (something like that).

I'll try something next week if nobody finds a solution in between.

[GeoSot]

Hello guys. Sorry for the delay, I have pushed a possible solution. Any feedback accepted

[umer936]

I have this same issue in a different context.

In BS 5.3.0, I was able to have a collapse with id "QueueableTasks.Queue". This is desired because it is in reference to CakePHP standard of [Plugin].[controller].

Example:
https://codepen.io/umer936-the-looper/pen/wvNJYqQ

I do not want to remove the period if possible.