Skip to content

Commit

Permalink
Tabs: Fixed detection of local vs. remote tabs. Fixes #4941 - Mishand…
Browse files Browse the repository at this point in the history
…ling of base tag. Fixes #4836 - Self refering href only partially detected.
  • Loading branch information
scottgonzalez committed Aug 11, 2011
1 parent ac04462 commit 18a3b53
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 31 deletions.
2 changes: 1 addition & 1 deletion tests/unit/tabs/tabs_core.js
Expand Up @@ -48,7 +48,7 @@ test( "aria-controls", function() {
tabs = element.find( ".ui-tabs-nav a" );
tabs.each(function() {
var tab = $( this );
equal( tab.attr( "href" ).substring( 1 ), tab.attr( "aria-controls" ) );
equal( tab.prop( "hash" ).substring( 1 ), tab.attr( "aria-controls" ) );
});

element = $( "#tabs2" ).tabs();
Expand Down
50 changes: 20 additions & 30 deletions ui/jquery.ui.tabs.js
Expand Up @@ -18,6 +18,19 @@ function getNextTabId() {
return ++tabId;
}

var isLocal = (function() {
var rhash = /#.*$/,
currentPage = location.href.replace( rhash, "" );

return function( anchor ) {
// clone the node to work around IE 6 not normalizing the href property
// if it's manually set, i.e., a.href = "#foo" kills the normalization
anchor = anchor.cloneNode();

This comment has been minimized.

Copy link
@monoblaine

monoblaine Aug 23, 2011

in addition to my comment below, this line gives the error in opera

return anchor.hash.length > 1 &&
anchor.href.replace( rhash, "" ) === currentPage;
};
})();

$.widget( "ui.tabs", {
version: "@VERSION",
options: {
Expand Down Expand Up @@ -197,8 +210,7 @@ $.widget( "ui.tabs", {
},

_processTabs: function() {
var self = this,
fragmentId = /^#.+/; // Safari 2 reports '#' for an empty hash
var self = this;

this.list = this.element.find( "ol,ul" ).eq( 0 );
this.lis = $( " > li:has(a[href])", this.list );
Expand All @@ -208,40 +220,21 @@ $.widget( "ui.tabs", {
this.panels = $( [] );

this.anchors.each(function( i, a ) {
var href = $( a ).attr( "href" ),
hrefBase = href.split( "#" )[ 0 ],
selector,
panel,
baseEl;

// For dynamically created HTML that contains a hash as href IE < 8 expands
// such href to the full page url with hash and then misinterprets tab as ajax.
// Same consideration applies for an added tab with a fragment identifier
// since a[href=#fragment-identifier] does unexpectedly not match.
// Thus normalize href attribute...
if ( hrefBase && ( hrefBase === location.toString().split( "#" )[ 0 ] ||
( baseEl = $( "base" )[ 0 ]) && hrefBase === baseEl.href ) ) {
href = a.hash;
a.href = href;
}
var selector, panel;

// inline tab
if ( fragmentId.test( href ) ) {
selector = href;
if ( isLocal( a ) ) {
selector = a.hash;
panel = self.element.find( self._sanitizeSelector( selector ) );
// remote tab
// prevent loading the page itself if href is just "#"
} else if ( href && href !== "#" ) {
} else {
var id = self._tabId( a );
selector = "#" + id;
panel = self.element.find( selector );
if ( !panel.length ) {
panel = self._createPanel( id );
panel.insertAfter( self.panels[ i - 1 ] || self.list );
}
// invalid tab href
} else {
self.options.disabled.push( i );
}

if ( panel.length) {
Expand Down Expand Up @@ -525,21 +518,18 @@ $.widget( "ui.tabs", {
options = this.options,
anchor = this.anchors.eq( index ),
panel = self._getPanelForTab( anchor ),
// TODO until #3808 is fixed strip fragment identifier from url
// (IE fails to load from such url)
url = anchor.attr( "href" ).replace( /#.*$/, "" ),
eventData = {
tab: anchor,
panel: panel
};

// not remote
if ( !url ) {
if ( isLocal( anchor[ 0 ] ) ) {
return;
}

this.xhr = $.ajax({
url: url,
url: anchor.attr( "href" ),
beforeSend: function( jqXHR, settings ) {
return self._trigger( "beforeLoad", event,
$.extend( { jqXHR : jqXHR, ajaxSettings: settings }, eventData ) );
Expand Down

2 comments on commit 18a3b53

@monoblaine
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

something in this commit causes opera to give errors. tabs are not rendered, and opera gives an error in _processTabs method

@scottgonzalez
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, fixed in 87f7897. Opera seems to be the only browser that enforces the deep parameter being required.

Please sign in to comment.