Ticket #5069 (closed bug: fixed)

Opened 5 years ago

Last modified 2 years ago

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:
Blocking: Blocked by:

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

comment:1 Changed 5 years ago by scott.gonzalez

  • Milestone changed from TBD to 1.8

comment:2 Changed 5 years ago by snover

comment:3 Changed 4 years ago by ajpiano

  • Priority changed from minor to critical

comment:4 Changed 4 years ago by dominiquevincent

comment:5 follow-up: ↓ 6 Changed 4 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 4 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 4 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 4 years ago by eric.muyser

Just ran into this bug, quite a pest.

comment:9 Changed 4 years ago by scott.gonzalez

  • Priority changed from critical to major

comment:10 Changed 4 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 4 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 4 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 3 years ago by petersendidit

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

comment:14 Changed 3 years ago by scott.gonzalez

  • Status changed from new to closed
  • Resolution set to fixed

comment:15 Changed 3 years ago by scott.gonzalez

#7570 is a duplicate of this ticket.

comment:16 Changed 3 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 follow-up: ↓ 18 Changed 3 years ago by scott.gonzalez

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 2 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 2 years ago by tadudek (previous) (diff)

comment:19 follow-up: ↓ 20 Changed 2 years ago by scott.gonzalez

comment:20 in reply to: ↑ 19 Changed 2 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.