Skip to content

Commit

Permalink
Tabs: Use CSS.escape for sanitizing selectors
Browse files Browse the repository at this point in the history
The previous private `_sanitizeSelector` API was not correctly escaping
backslashes and is now removed. The native API should always be correct.

Closes gh-2307
  • Loading branch information
mgol authored Oct 26, 2024
1 parent ebdcd0d commit af8adca
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 12 deletions.
31 changes: 31 additions & 0 deletions tests/unit/tabs/core.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,37 @@ QUnit.test( "non-tab list items", function( assert ) {
"first actual tab is active" );
} );

QUnit.test( "ID escaping backslashes", function( assert ) {
assert.expect( 5 );

location.hash = "#fragment\b-2";

var element = $( "#tabs1" )
.find( "a[href='#fragment-2']" )
.attr( "href", "#fragment\b-2" )
.end()
.find( "#fragment-2" )
.attr( "id", "fragment\b-2" )
.end()
.tabs();
var tabs = element.find( ".ui-tabs-nav li" );
var anchors = tabs.find( ".ui-tabs-anchor" );
var panels = element.find( ".ui-tabs-panel" );

assert.strictEqual( element.tabs( "option", "active" ), 1,
"should set the active option" );

assert.strictEqual( anchors.length, 3, "should decorate all anchors" );
assert.strictEqual( panels.length, 3, "should decorate all panels" );

assert.strictEqual( panels.eq( 1 ).attr( "aria-labelledby" ), anchors.eq( 1 ).attr( "id" ),
"panel 2 aria-labelledby equals anchor 2 id" );
assert.strictEqual( tabs.eq( 1 ).attr( "aria-controls" ), "fragment\b-2",
"tab 2 aria-controls" );

location.hash = "";
} );

QUnit.test( "aria-controls", function( assert ) {
assert.expect( 7 );
var element = $( "#tabs1" ).tabs(),
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/tabs/helper.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ return $.extend( helper, {
var expected = $.makeArray( arguments ).slice( 2 ),
actual = tabs.find( ".ui-tabs-nav li" ).map( function() {
var tab = $( this ),
panel = $( $.ui.tabs.prototype._sanitizeSelector(
"#" + tab.attr( "aria-controls" ) ) ),
panel = $( "#" + CSS.escape( tab.attr( "aria-controls" ) ) ),
tabIsActive = tab.hasClass( "ui-state-active" ),
panelIsActive = panel.css( "display" ) !== "none";

Expand Down
16 changes: 6 additions & 10 deletions ui/widgets/tabs.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,14 +121,14 @@ $.widget( "ui.tabs", {
_initialActive: function() {
var active = this.options.active,
collapsible = this.options.collapsible,
locationHash = location.hash.substring( 1 );
locationHashDecoded = decodeURIComponent( location.hash.substring( 1 ) );

if ( active === null ) {

// check the fragment identifier in the URL
if ( locationHash ) {
if ( locationHashDecoded ) {
this.tabs.each( function( i, tab ) {
if ( $( tab ).attr( "aria-controls" ) === locationHash ) {
if ( $( tab ).attr( "aria-controls" ) === locationHashDecoded ) {
active = i;
return false;
}
Expand Down Expand Up @@ -312,10 +312,6 @@ $.widget( "ui.tabs", {
}
},

_sanitizeSelector: function( hash ) {
return hash ? hash.replace( /[!"$%&'()*+,.\/:;<=>?@\[\]\^`{|}~]/g, "\\$&" ) : "";
},

refresh: function() {
var options = this.options,
lis = this.tablist.children( ":has(a[href])" );
Expand Down Expand Up @@ -434,9 +430,9 @@ $.widget( "ui.tabs", {

// Inline tab
if ( that._isLocal( anchor ) ) {
selector = anchor.hash;
selector = decodeURIComponent( anchor.hash );
panelId = selector.substring( 1 );
panel = that.element.find( that._sanitizeSelector( selector ) );
panel = that.element.find( "#" + CSS.escape( panelId ) );

// remote tab
} else {
Expand Down Expand Up @@ -874,7 +870,7 @@ $.widget( "ui.tabs", {

_getPanelForTab: function( tab ) {
var id = $( tab ).attr( "aria-controls" );
return this.element.find( this._sanitizeSelector( "#" + id ) );
return this.element.find( "#" + CSS.escape( id ) );
}
} );

Expand Down

0 comments on commit af8adca

Please sign in to comment.