Opened 14 years ago

Closed 12 years ago

Last modified 11 years ago

#5069 closed bug (fixed)

ui.tabs.add creates two tab panels when using a full URL

Reported by: snover Owned by:
Priority: major Milestone: 1.9.0
Component: ui.tabs Version: 1.8a2
Keywords: tabify, tabId, add Cc:
Blocked by: Blocking:

Description

When creating a new tab by passing in a full URL to ui.tabs.add, two tab panels are added to the DOM—one by ui.tabs.add and another by ui.tabs._tabify. The panel created by ui.tabs.add never gets used, never gets removed, and ends up sitting wasting space for no reason.

I’m not familiar enough yet with the internals of jQuery UI to know what the proper solution is; removing all references to $panel in ui.tabs.add gets things working somewhat better for AJAX calls, but breaks page fragments completely.

The generated ID also ends up double-incremented because ui.tabs._tabId gets called twice: once to get the id for the tab in ui.tabs.add, and then again to get a tab panel ID in ui.tabs._tabify.

Regards,

Change History (21)

comment:1 Changed 14 years ago by Scott González

Milestone: TBD1.8

comment:3 Changed 13 years ago by ajpiano

Priority: minorcritical

comment:4 Changed 13 years ago by dominiquevincent

comment:5 Changed 13 years ago by dominiquevincent

A proposal to fix the problem is available here : http://github.com/dominiquevincent/jquery-ui/commit/8f69b7546253eb9328b8e72549ff2a7c9d4b2db9

The panel in ui.tabs.add is created only if it's an in-page tab.

2 html files have been added in tests/visual/tabs in order to test this fix.

comment:6 in reply to:  5 Changed 13 years ago by Jelly

Replying to dominiquevincent:

A proposal to fix the problem is available here : http://github.com/dominiquevincent/jquery-ui/commit/8f69b7546253eb9328b8e72549ff2a7c9d4b2db9

The panel in ui.tabs.add is created only if it's an in-page tab.

2 html files have been added in tests/visual/tabs in order to test this fix.

Only one element is added to the DOM but the _tabId function is called twice and next tab gets double-incremented id.

First tab called: ui-tabs-2

Second tab: ui-tabs-4

comment:7 Changed 13 years ago by jquery@…

I've solved this by changing the _tabId function:

_tabId: function(a) {
        //PATCH ADDITION, if id set in data use that (when adding a tab), otherwise tabify will create two panels for 1 added tab
        if($.data(a, 'id')) {
            return $.data(a, 'id');
        } else {
        return a.title && a.title.replace(/\s/g, '_').replace(/[^A-Za-z0-9\-_:\.]/g, '') ||
            this.options.idPrefix + getNextTabId();
        }
    }

And in the add function adding the following line:

add: function(url, label, index) {
        if (index === undefined) {
            index = this.anchors.length; // append by default
        }

        var self = this, o = this.options,
            $li = $(o.tabTemplate.replace(/#\{href\}/g, url).replace(/#\{label\}/g, label)),
            id = !url.indexOf('#') ? url.replace('#', '') : this._tabId($('a', $li)[0]);
        //PATCH ADDITION, set id
        $.data($('a', $li)[0], 'id', id);
        $li.addClass('ui-state-default ui-corner-top').data('destroy.tabs', true);

        // try to find an existing element before creating a new one
        var $panel = $('#' + id);

comment:8 Changed 13 years ago by eric.muyser

Just ran into this bug, quite a pest.

comment:9 Changed 13 years ago by Scott González

Priority: criticalmajor

comment:10 Changed 13 years ago by lotjuh

I solved this by changing only the add function (removed lines with ):

    add: function( url, label, index ) {
        if ( index === undefined ) {
            index = this.anchors.length;
        }

        var self = this,
            o = this.options,
            $li = $( o.tabTemplate.replace( /#\{href\}/g, url ).replace( /#\{label\}/g, label ) );

        //id = !url.indexOf( "#" ) ? url.replace( "#", "" ) : this._tabId( $( "a", $li )[ 0 ] );

        $li.addClass( "ui-state-default ui-corner-top" ).data( "destroy.tabs", true );

        // try to find an existing element before creating a new one
    //  var $panel = $( "#" + id );
    //  if ( !$panel.length ) {
    //      $panel = $( o.panelTemplate )
    //          .attr( "id", id )
    //          .data( "destroy.tabs", true );
    //  }
    //  $panel.addClass( "ui-tabs-panel ui-widget-content ui-corner-bottom ui-tabs-hide" );

        if ( index >= this.lis.length ) {
            $li.appendTo( this.list );
            //$panel.appendTo( this.list[ 0 ].parentNode );
        } else {
            $li.insertBefore( this.lis[ index ] );
        //  $panel.insertBefore( this.panels[ index ] );
        }

        o.disabled = $.map( o.disabled, function( n, i ) {
            return n >= index ? ++n : n;
        });

        this._tabify();

        if ( this.anchors.length == 1 ) {
            o.selected = 0;
            $li.addClass( "ui-tabs-selected ui-state-active" );
            $panel.removeClass( "ui-tabs-hide" );
            this.element.queue( "tabs", function() {
                self._trigger( "show", null, self._ui( self.anchors[ 0 ], self.panels[ 0 ] ) );
            });

            this.load( 0 );
        }

        this._trigger( "add", null, this._ui( this.anchors[ index ], this.panels[ index ] ) );
        return this;
    },

comment:11 Changed 13 years ago by iainxt

Until it is fixed, I am using the following workaround. It seems to work for me. If you expect the active div to be empty, then it wont work.

tabContainer.bind("tabsload",function(){
  $(this).children('div.ui-tabs-panel:empty').addClass('ui-tabs-hide');
 });

comment:12 Changed 13 years ago by stefanlivens

I just found out the hard way this bug exists, but I also stumbled upon a workaround.

The point is, if you it like this:

<div id="tabs">
   <ul>...</ul>
   <div id="tab-1">
   </div>
   ...
   <div id="tab-new">
      ...content...
      <script>
      $(function() {
   	$("#tabs").tabs( "add" , "#tab-new" , "New added tab" );
      });
      </script>
   </div>
</div>

Then you'll get 2 "new added tab"'s in the tabbar!

If you do it like this:

<div id="tabs">
   <ul>...</ul>
   <div id="tab-1">
   </div>
   ...
   <div id="tab-new">
      ...content...
   </div>
   <script>
   $(function() {
      $("#tabs").tabs( "add" , "#tab-new" , "New added tab" );
   });
   </script>
</div>

Then it's ok. (the <script> is outside of the div that becomes a tab-panel)

It looks like the content of the tab is "executed" again when adding it like a tab.

comment:13 Changed 12 years ago by petersendidit

This looks to be fixed with the new tabs rewrite: http://jsfiddle.net/petersendidit/rmEYT/17/

comment:14 Changed 12 years ago by Scott González

Resolution: fixed
Status: newclosed

comment:15 Changed 12 years ago by Scott González

#7570 is a duplicate of this ticket.

comment:16 Changed 12 years ago by john.culviner

Hi,

I'm curious about the status of this bug. I have latest jQuery UI @ 1.8.16 on the bug still exists. I would be willing to switch to the "tabs rewrite" but where is that? I'd rather not go into the source and fix it to stay future compatible. The "Fixed in 1bacdec6be7e9d543224c69344041aaccde059c0" has a tooltip that says "No changeset 1bacdec6be7e9d543224c69344041aaccde059c0 exists in the repository." so I'm wondering if this got lost somewhere by mistake?

Thanks for the help!

comment:17 Changed 12 years ago by Scott González

The milestone says 1.9, so this is not in 1.8.16. You need to use master to get this fix.

comment:18 in reply to:  17 Changed 11 years ago by tadudek

Replying to scott.gonzalez:

The milestone says 1.9, so this is not in 1.8.16. You need to use master to get this fix.

-Where do I find master?

EDIT: found it, nm.

Last edited 11 years ago by tadudek (previous) (diff)

comment:19 Changed 11 years ago by Scott González

comment:20 in reply to:  19 Changed 11 years ago by tadudek

Replying to scott.gonzalez:

http://github.com/jquery/jquery-ui

is there a minified version of the pre-release?

Note: See TracTickets for help on using tickets.