Tabs: Use CSS.escape for sanitizing selectors by mgol · Pull Request #2307 · jquery/jquery-ui (original) (raw)
The previous private _sanitizeSelector API was not correctly escaping backslashes and is now removed. The native API should always be correct.
Comment on lines -315 to -317
| _sanitizeSelector: function( hash ) { |
|---|
| return hash ? hash.replace( /[!"$%&'()*+,.\/:;<=>?@\[\]\^`{|}~]/g, "\\$&" ) : ""; |
| }, |
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fnagel It's internal only so I thought we can just remove it. Or should we be conservative & only deprecate in 1.14? I don't know how jQuery UI historically approached removing private widget methods.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see there's at least one case of $.fn._focus getting renamed to $.fn.focus between 1.10.1 & 1.10.2: 1.10.1...1.10.2
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ohh that is in indeed a good question. Iirc Scott would not make a big thing of changing internal methods but I'm not really sure.
Why not just keep the $.fn._sanitizeSelector$.ui.tabs.prototype._sanitizeSelector until the next minor release but make it use CSS.escape?
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just seen its not $.fn but $.ui.tabs so it's even more specific (as in Tabs widget only). Not sure if its worth keeping the old method around...
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, I'll keep it removed then. There's no universal way to sanitize the whole selector; the safe thing to do is to just escape the identifier part, especially when potentially coming from user input. Even if I changed the internal method to use CSS.escape, it would just handle the "# + id" case, which can be confusing and can still lead people to use it where it's not intended.
The previous private _sanitizeSelector API was not correctly escaping
backslashes and is now removed. The native API should always be correct.
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Ref jquerygh-2307
mgol added a commit to mgol/jquery-ui that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
jquerygh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.
Fixes jquerygh-2344 Closes jquerygh-2345 Ref jquerygh-2307
mgol added a commit that referenced this pull request
Prior to jQuery UI 1.14.1, hashes in anchor hrefs were used directly. In
gh-2307, that was changed - by decoding - to support more complex IDs, e.g.
containing emojis which are automatically encoded in anchor.hash.
Unfortunately, that broke cases where the panel ID is decoded as well.
It turns out the spec mandates checking both. In the "scrolling to a fragment" section of the HTML spec^1. That uses a concept of document's indicated part^2. Slightly below there's an algorithm to compute the indicated part^3. The interesting parts are steps 4 to 9: 4. Let potentialIndicatedElement be the result of finding a potential indicated element given document and fragment. 5. If potentialIndicatedElement is not null, then return potentialIndicatedElement. 6. Let fragmentBytes be the result of percent-decoding fragment. 7. Let decodedFragment be the result of running UTF-8 decode without BOM on fragmentBytes. 8. Set potentialIndicatedElement to the result of finding a potential indicated element given document and decodedFragment. 9. If potentialIndicatedElement is not null, then return potentialIndicatedElement.
First, in steps 4-5, the algorithm tries the hash as-is, without decoding. Then, if one is not found, the same is attempted with a decoded hash.
This change replicates this logic by first trying the hash as-is and then decoding it.