-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixes issue 1537 - OOB does not escape query selector #2319
base: dev
Are you sure you want to change the base?
Conversation
@@ -799,7 +799,7 @@ return (function () { | |||
* @returns | |||
*/ | |||
function oobSwap(oobValue, oobElement, settleInfo) { | |||
var selector = "#" + getRawAttribute(oobElement, "id"); | |||
var selector = '[id="' + getRawAttribute(oobElement, "id") + '"]'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't be nicer to use CSS.escape()?
let selector = '#' + CSS.escape(getRawAttribute(oobElement, 'id'))
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it would be nicer...but as I understood it htmx v1 maintains support for Internet Explorer 11 and CSS.escape
isn't supported.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm I am curious how this fix will be propagated to v2... Contribution rules says PR only into dev.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @FraserChapman , following up late on this (sorry about that!)
Since then, we released htmx 2 which dropped IE11 support, so feel free to use that method if you want 😄 |
Description
This change alters how the oobSwap function builds the query selector string. Rather than simply prefixing it with a
#
it uses the'[id="raw_id"]'
format. Whereraw_id
is the value obtained fromgetRawAttribute(oobElement, "id")
Ideally this could be done more cleanly with template literals (backtick strings) - but as I understand it you want to support IE11.
This fix allows the call querySelectorAll to return the nodelist of all elements with the ids such as "foo-/bar" - without getting "Failed to execute 'querySelectorAll' on 'Document': '#foo-/bar' is not a valid selector, and whilst not limiting the selection to the first matching id in the document.
Corresponding issue:
#1537
Testing
I ran the htmx.js test suite Version: 1.9.10 and got 0 failures
I also manually tested ids such as "foo-/bar" and it all seems to be working as expected.
Further to this I've added test cases for
'handles elements with IDs containing special characters properly'
and
'handles one swap into multiple elements with the same ID properly'
And re- ran the htmx.js test suite Version: 1.9.10 and got 0 failures
Checklist
master
for website changes,dev
forsource changes)
approved via an issue
npm run test
) and verified that it succeeded