Skip to content

Fix accordion and tab targets with special meaning characters - #14789

Closed
jaskorpe wants to merge 2 commits into
twbs:masterfrom
jaskorpe:fix-accordion-tab-target-selector
Closed

jaskorpe wants to merge 2 commits into
twbs:masterfrom
jaskorpe:fix-accordion-tab-target-selector

Conversation

@jaskorpe

Copy link
Copy Markdown

This fixes the issue of special meaning characters in id's on togglable tabs, and collapsible panels without the need for escaping the id's.

Refs: #14285, #13832

Issue twbs#13832 suggests escaping the selector, but this issue is fixed
much more cleanly with an attribute selector. Because bootstrap
documentation uses href's with hash links, this patch allows links
both with and without leading hash
Issue twbs#13832 suggests escaping the selector, but this issue is fixed
much more cleanly with an attribute selector. Because bootstrap
documentation uses href's with hash links, this patch allows links
both with and without leading hash
@cvrebert cvrebert added the js JavaScript or TypeScript sources and plugins label Oct 12, 2014
@cvrebert

Copy link
Copy Markdown
Collaborator
  • There are some additional places that would also need to be changed: see Escape selectors #13832 (comment)
  • There are possibly performance concerns with this approach. Do browsers optimize id attribute selectors like they do #id selectors? Does it take jQuery longer to parse these attribute selectors?
  • The Travis build is failing due to some apparent breakage in the unit tests
  • New unit tests should be added to specifically test the handling of "exotic" IDs

@hnrch02

hnrch02 commented Oct 13, 2014

Copy link
Copy Markdown
Collaborator

Does it take jQuery longer to parse these attribute selectors?

Not sure about parsing but Sizzle uses the much faster getElementById when # is matched, see https://github.com/jquery/sizzle/blob/9bf8ecbc8d39e436cdedd23be5f1843e2587c507/src/sizzle.js#L208.

Anyways, as I said in #13832 (comment), support for these "exotic" IDs is planned for v4 so I'd rather not go this route for v3.

@jaskorpe

Copy link
Copy Markdown
Author

I did see that you wanted to wait, but I didn't see any mention of attribute selectors in the proposed patch. If there indeed is a performance penalty, I agree that it is best not to merge these patches.

The patches also introduces some uncertainty in the api since the documentation suggests using # as href value, and then allowing the id to start with a hash.

@hnrch02

hnrch02 commented Oct 22, 2014

Copy link
Copy Markdown
Collaborator

I will close this as we're unlikely to add support for this in v3.
As I said, support for these IDs is planned for v4 when we'll hopefully have a better infrastructure for common utilities across the plugins, so you just have to use normal IDs until then. Thanks for the work though!

@ktmud

ktmud commented Dec 28, 2015

Copy link
Copy Markdown

Another weird case is IDs with a colon. For example, Hugo will generate section anchors with IDs like #SOME_TITLE:13f5f3bf.

While this is a valid "href" attribute for an anchor, passing it to jQuery directly will not find the element. However, $("#SOME_TITLE\\:13f5f3bf") will work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

js JavaScript or TypeScript sources and plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants