-
-
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
OOB does not escape query selector #1537
Comments
Is that a valid |
@alexpetros According to MDN, it is:
The documentation for querySelector actually mentions escaping special characters though it surprisingly does not mention using CSS.escape which would be the easiest way to handle the above. I'll try and create a PR for this. |
I still think this is a reasonable request if you want to write your own escape function or just wait for htmx 2. |
what about using getElementByID when it is detected to be an ID? that method is able to work with any id string |
Escape OOB query selector to support special characters in element ids. Refs bigskysoftware#1537
This problem doesn't just apply to <button hx-get="/clicked" hx-swap="beforeend" hx-target="#target!">Click me</button>
<div id="target!"></div> That makes the solution more complicated, as we can't simply apply some escape function inside f.i. I implemented a simple escape function to test this (here) and found many failing tests when also applying this escape function inside I don't quite get where the code that processes those attributes calls Either way, in my commit above I implemented a simple escape function that fixes the issue for |
Yes, this! The OOB swapping function uses Other attributes target elements by specifying an actual selector, not an ID like |
I was tempted to have a bash at this, but just wanted to clarify/confuse(!?) the issue.
This is not entirely accurate. While it's true that, in practical terms, each That is, each This concept is outlined in the HTML5 specification [1][2], where the "home subtree" of an element is defined as the portion of the DOM tree rooted at the element's root element. Therefore, within different subtrees of the document, elements can have the same To illustrate this point, consider the markup of websites like YouTube, where multiple elements with the same For a practical demonstration, you can visit a YouTube video page (e.g., https://www.youtube.com/watch?v=LriHRa9t1fQ) and open the browser console. Running This works because subtrees form their own scope, the best example of this being the "Shadow DOM" which is an adjunct tree of DOM nodes. Thus any such subtree can contain ID that overlap with IDs in the rest of the document, without the the IDs in the subtree clashing with those in the document. Or in a simpler contrived example The practical upshot of this in regards to this issue is that you can do - References: [1] HTML5 Specification: The ID Attribute (https://www.w3.org/TR/2011/WD-html5-20110525/elements.html#the-id-attribute) [2] HTML5 Specification: Home Subtree (https://www.w3.org/TR/2011/WD-html5-20110525/infrastructure.html#home-subtree) |
Have popped a PR in to address this based on my previous comment - #2319 |
@FraserChapman your explanation of home subtree is incorrect. With any node in the same Document, the home subtree with be that Document. In your example, it means all of those IDs' home subtrees are the same: Note: as you said, using an ID several times is however valid with each ID in a different subtree like different Shadow DOMs. However, you seem to misunderstand the concept of subtree: with a single ID per document, any
Taking Youtube (or any production website) as an example to prove a point regarding browser standards is generally a bad idea. Unfortunately browsers won't mark this as an The initial problem@eduardvercaemer It is a very bad idea to use any random character in IDs: it will break in certain browsers, document fragment links will not be well-supported, and so on. For the sake of sanity checking, my honest opinion would be that the maintainers look at the issue again and reflect about why the solution for your problem would not simply be to refrain from using weird characters in the ID 😅 @matiboy if you had linked the entire MDN paragraph, it would have simplified the discussion:
|
@Atmos4i to clarify, that's not my explanation, that is the wording of the specification.
The example I provided was, as I stated, contrived to illustrate the discussion, it was not meant to be a complaint or respect w3. Further the YouTube example was to demonstrate the use of subtrees "in the wild" - where multiple ids with the same value exist, it wasn't to "prove a point about browser standards". My understanding of subtrees was
Could you point out where my failure of understanding is? It's entirely posible I'm misunderstanding something, but I'm not clear what you are referring to here. Leaving aside the debate around multiple ids with the same value, my main point about querySelectorAll with regard to this issue was that it can be used to select the correct element without the need for escaping (the entire basis of this discussion)...so saying my whole point about querySelectorAll being incorrect, is ironically, itself incorrect :) The further point I was making about multiple ids with the same value was that it would work on sites "in the wild" where document.querySelectorAll("some-id") does in fact return a NodeList of multiple elements. Apologies if I confused any of this, as I said it's entirely possible I'm misunderstanding certain aspects of this :) I do see you point about not supporting certain characters as an easy "solution" - but to a certain extent doesn't this amount to "we don't support certain valid IDs because it's too difficult"? |
Let me try to explain: The term
I made a quick Codepen example to demonstrate this.
The example you provided is only involving
The Youtube website is the exact same as your example.
Adding support for this is very simple, like you show in your PR: I hope I did not sound too off-putting. I am merely trying to:
|
Ok I think I see the misunderstanding, my simple contrived example really wasn't being used to illustrate the concept of a subtree - it was being used to illustrate how querySelectorAll does, in fact, return a node list of multiple elements if the elements have the same id. That is all. I apologise if I have confused the discussion here. Granted I misunderstood youtube was ultimately the same thing, I thought this was doing something different. So again apologies if this confused things. If all valid IDs are to be supported, I suppose the wider issue would be when multiple ids are supplied such as "#foo #bar/baz #bat!" - with regards to
Has to be better than doing... document.getElementById('foo') And then aggregating the results of the separate calls.
I do actually have a working branch that implements this for Also not off-putting at all, I think we are all trying to best understand, clarify and find reasonable solutions :) With that in mind, I hope I can say, without sounding condescending, I do still find the proposed...
...a bit of a fudge. My point being that ultimately the IDs are valid according to the specification, and it is maintainable with a bit of thought and care. However this is obviously a debatable point! I'll post a PR that illustrates the wider point about |
To follow up to this, re the comment from @Encephala - #1537 (comment)
My idea was that one could extend the
In code this would look something like... ...
} else if (selector.indexOf("#") !== -1) {
var formattedSelectors = selector.split(",").map(function (s) {
var normalized = normalizeSelector(s);
return normalized.indexOf("#") === 0 ?
'[id="' + normalized.substr(1) + '"]' :
normalized;
}).join(",");
return getDocument().querySelectorAll(formattedSelectors);
} else {
return getDocument().querySelectorAll(normalizeSelector(selector));
} Then given a range of possible "selector" strings, the "formattedSelectors" results would be...
etc... Thus allowing for a single call to However based on based @Atmos4 various comments it looks like this might just have all been a waste of time... If that is the case possibly remove the "help wanted" tag and close this issue to save any other well meaning people wasting their time trying to implement a solution for valid IDs that aren't going to be supported in any circumstance. Thanks for the responses and clarifications, and again my apologies for any confusion I introduced in the discussion. I was genuinely just trying to help find a solution to the root issue raised and the wider issue with |
Nothing is ever a waste of time 🙂 discussing a subject and fool-proofing ideas is not only super valuable, I would argue it is a building block of OSS. So keep doing what you're doing, because you are doing great. However, sometimes there is a need to take a step back. For example here:
Regarding the argument with IDs: I was just surprised that MDN recommendations regarding ID format were not fully mentioned, and I was also trying to fool-proof that big message about home subtree isolation which was a bit wrong. But about the topic of the issue, I'd say the PR you mentioned makes sense 👍 |
@Atmos4 Yes, the root issue was FWIW I'd also been playing with
Which would be resolved by the
However I should probably just wait until I'm at my computer - as I'm writing this all from memory, on my phone, whilst on a train :/ |
I have a very simple HTML with some elements such as
I want to execute an OOB swap, but sending the following HTML
results in the following error in the console
The query selector can be escaped like '#foo-\/bar\/' and should be done automatically by htmx.
The text was updated successfully, but these errors were encountered: