Opened 10 years ago

Closed 8 years ago

#5485 closed bug (wontfix)

ajax tabs event (tabsshow and tabsload) order and trigger

Reported by: d.duponchel Owned by:
Priority: major Milestone: 1.9.0
Component: ui.tabs Version: 1.8
Keywords: event trigger Cc:
Blocked by: Blocking:

Description

With ajax tabs :

  • the tabsshow event is triggered before the tabsload (the documentation says "tabsselect, tabsload, tabsshow (in that order)")
  • for the first tab, the tabsshow event isn't triggered (but the show method is).



Reproducible : always (see the attached html file)

When loading the page and clicking on the second tab, we have :

show 0
tabsload 0
load 0
tabsshow 1
show 1
tabsload 1
load 1

expected :

tabsload 0
load 0
tabsshow 0
show 0
tabsload 1
load 1
tabsshow 1
show 1

Tested browser : firefox 2/3/3.5/3.6, opera 10, IE 7/8, safari 4, chrome 4.

Attachments (3)

1.html (2 bytes) - added by d.duponchel 10 years ago.
dummy file for 5485
2.html (2 bytes) - added by d.duponchel 10 years ago.
dummy file for 5485
jQueryUI-5485.html (1.3 KB) - added by d.duponchel 10 years ago.
updated test file

Download all attachments as: .zip

Change History (11)

Changed 10 years ago by d.duponchel

Attachment: 1.html added

dummy file for 5485

Changed 10 years ago by d.duponchel

Attachment: 2.html added

dummy file for 5485

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

Milestone: TBD1.9
Priority: majorcritical

comment:2 Changed 10 years ago by d.duponchel

The second point (tabsshow event isn't triggered) is a stupid mistake (.tabs() before .bind()). Still, the first / main issue (event order) remains true.
I can't find any "edit" button, so here is an updated trace :
got :

tabsshow 0
show 0
tabsload 0
load 0
tabsshow 1
show 1
tabsload 1
load 1

expected :

tabsload 0
load 0
tabsshow 0
show 0
tabsload 1
load 1
tabsshow 1
show 1

Changed 10 years ago by d.duponchel

Attachment: jQueryUI-5485.html added

updated test file

comment:3 Changed 9 years ago by dominiquevincent

Using the attachment file, a unit test has been made. It is available here : http://github.com/dominiquevincent/jquery-ui/commit/6004a6294746d238c0330a6af4a9f441ee379879

The issue seems to be fixed.

comment:4 Changed 9 years ago by dominiquevincent

The visual test (tabsEventsOrder.html) used a full path to css file, I replace it by relative path.

http://github.com/dominiquevincent/jquery-ui/commit/f2e42c5911b21ca8b38204fd897f79c76a85d9c5

comment:5 Changed 9 years ago by d.duponchel

That's strange : on firefox, the tests pass only when the external html files (tabs content) are missing, and when there is no server (file://). Otherwise (the tab content exists or there is an http server) the tests fail. I've updated the unit test to highlight the problem : http://github.com/dduponchel/jquery-ui/commit/0349648f3cba918f667a818cd28c3b0d2aa19233

comment:6 Changed 9 years ago by milan

I think this was broken in a5eab4cc38c73c3c54b9c6a6d071ad93263f50f5 to fix #4451.

in the load event for tabs the "tabs" queue is dequeued outside of the $.ajax loader. I think if you move

// last, so that load event is fired before show...
self.element.dequeue( "tabs" );

to the bottom of the load success: i.e.

load: function( index ) {
	index = this._getIndex( index );
	var self = this,
		o = this.options,
		a = this.anchors.eq( index )[ 0 ],
		url = $.data( a, "load.tabs" );

	this.abort();

	// not remote or from cache
	if ( !url || this.element.queue( "tabs" ).length !== 0 && $.data( a, "cache.tabs" ) ) {
		this.element.dequeue( "tabs" );
		return;
	}

	// load remote from here on
	this.lis.eq( index ).addClass( "ui-state-processing" );

	if ( o.spinner ) {
		var span = $( "span", a );
		span.data( "label.tabs", span.html() ).html( o.spinner );
	}

	this.xhr = $.ajax( $.extend( {}, o.ajaxOptions, {
		url: url,
		success: function( r, s ) {
			$( self._sanitizeSelector( a.hash ) ).html( r );

			// take care of tab labels
			self._cleanup();

			if ( o.cache ) {
				$.data( a, "cache.tabs", true );
			}

			self._trigger( "load", null, self._ui( self.anchors[ index ], self.panels[ index ] ) );
			try {
				o.ajaxOptions.success( r, s );
			}
			catch ( e ) {}

	                // last, so that load event is fired before show...
	                self.element.dequeue( "tabs" );

		},
		error: function( xhr, s, e ) {
			// take care of tab labels
			self._cleanup();

			self._trigger( "load", null, self._ui( self.anchors[ index ], self.panels[ index ] ) );
			try {
				// Passing index avoid a race condition when this method is
				// called after the user has selected another tab.
				// Pass the anchor that initiated this request allows
				// loadError to manipulate the tab content panel via $(a.hash)
				o.ajaxOptions.error( xhr, s, index, a );
			}
			catch ( e ) {}

		}
	} ) );

	// last, so that load event is fired before show...
        // this is now dequeued after the tab has successfully loaded
	// self.element.dequeue( "tabs" );

	return this;
},

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

Priority: criticalmajor

comment:8 Changed 8 years ago by Scott González

Resolution: wontfix
Status: newclosed

This is handled much better with the new events, beforeload, load, beforeactivate, activate. We won't change the existing behavior from the old API, which is being deprecated.

Note: See TracTickets for help on using tickets.