Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#9166 closed bug (fixed)

Dialog: moveToTop implementation resets flash/video/iframe/scroll

Reported by: ardoramor Owned by:
Priority: blocker Milestone: 1.11.0
Component: ui.dialog Version: 1.10.2
Keywords: Cc:
Blocked by: Blocking:

Description

When multiple dialogs are open, each containing a flash or video element, and the dialogs are selected so that the other loses focus, UI code moves the newly selected dialog to the top by inserting all other sibling dialogs before the selected dialog. This causes the flash content of the deselected dialog to be reset as it is detached from DOM. The examples can be seen below:

http://jsfiddle.net/FgJKk/ <- JqueryUI v1.9.2, works fine http://jsfiddle.net/FgJKk/1/ <- JqueryUI v1.10.2, flash elements, breaks http://jsfiddle.net/FgJKk/2/ <- JqueryUI v1.10.2, video elements, breaks

Change History (23)

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

Milestone: none1.11.0
Priority: minorblocker
Status: newopen
Summary: UI.Dialog moveToTop implementation resets active flash / video elementsDialog: moveToTop implementation resets flash/video/iframe/scroll

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

#9167 is a duplicate of this ticket.

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

#9063 is a duplicate of this ticket.

comment:4 Changed 10 years ago by mpermana

The fix is to remove the mousedown handler, why is the code doing moveToTop when mouse is down? I think that's wrong. Please remove the code that call moveToTop in jquery-ui.js line 9874:

mousedown: function( event ) {

if ( this._moveToTop( event ) ) {

this._focusTabbable();

}

}

comment:5 in reply to:  4 Changed 10 years ago by Scott González

Replying to mpermana:

The fix is to remove the mousedown handler, why is the code doing moveToTop when mouse is down?

Because clicking on a dialog should bring it in front of other dialogs. That's not the problem. The problem is how moveToTop() is implemented, which changed in 1.10.0.

comment:6 Changed 10 years ago by aNt1X

i think that this bug relates to the one i opened in the past

http://bugs.jqueryui.com/ticket/9067

and marked as "cant fix"

Last edited 10 years ago by aNt1X (previous) (diff)

comment:7 in reply to:  6 Changed 10 years ago by Scott González

Replying to aNt1X:

i think that this bug relates to the one i opened in the past

Yes, I've repurposed this ticket to encompass all related tickets. The truth is that we can't fix this and simultaneously avoid z-index hell. We pushed for browser vendors to do the sane thing, but the response we've gotten is that it's too hard. We're going to look into how much z-index annoyance we're willing to live with.

comment:8 Changed 10 years ago by tvdeyen

This is a real blocker for us. But I see also that this is hard to fix, without going back to the old z-index implementation. Hard one :) Couldn't you at least introduce on option that disables the auto z-index mechanism?

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

No, we will never introduce an option to work around a current bug.

comment:10 Changed 10 years ago by aNt1X

Just to let the users know the workaround i've temporary used:

I have many dialogs, but only one of them contains an iframe, and it is "single instance".

So, I decided to rewrite the _moveToTop() function:

        _moveToTop : function (event, silent) {
            var moved = false;

            // *** WORKAROUND FOR THE FOLLOWING ISSUES ***
            //
            // New stacking feature causing iframe problems when changing active dialog
            // http://bugs.jqueryui.com/ticket/9067
            //
            // Moving an IFRAME within the DOM tree results in reloading of content
            // https://bugs.webkit.org/show_bug.cgi?id=13574

            if (this.uiDialog.nextAll(":visible").length > 0) {
                moved = true;

                if (this.uiDialog.nextAll(":visible").children('iframe').length > 0) {
                    // if one of the next elements contains an iframe, move this dialog after the last child
                    this.uiDialog.insertAfter(this.uiDialog.nextAll(":visible:last"));
                } else {
                    // standard behaviour: move the next elements before this one
                    this.uiDialog.nextAll(":visible").insertBefore(this.uiDialog);
                }
            }

            if (moved && !silent) {
                this._trigger("focus", event);
            }

            return moved;
        }

It's an ugly, ugly, hack: basically I try to not move the iframe's dialog, and move the other ones.

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

#9225 is a duplicate of this ticket.

comment:12 Changed 10 years ago by Bk1

I'm using jQuery UI to create a windowed desktop. All dialogs with scrolling elements are reset to top when _moveToTop is called. This problem is huge for me (and for my interface)...

At the moment I am trying to reimplement _moveToTop so that it stores the "scrollTop" of all involved scrollable elements and restore the scrollTop position after the dialog divs are swapped. It does not work very well, the code is horrible and it is specific to my case.

Honestly I don't know how to solve the problem in a clean way. I just stepped in to let know how important it is to find a solution. Thanks!

comment:13 Changed 10 years ago by Jörn Zaefferer

We discussed this in Portland. Scott was open to going back to some, hopefully limited, z-index shuffling. I don't remember the details though.

comment:14 Changed 10 years ago by cRz

The following part solves the problem..:

_moveToTop: function( event, silent ) {
	var $parent = this.uiDialog.parent();
	var $elementsOnSameLevel = $parent.children();
	
	var heighestZIndex = 0;
	$.each($elementsOnSameLevel, function(index, element) {
		var zIndexOfElement = $(element).css('z-index');
		if (zIndexOfElement) {
			var zIndexOfElementAsNumber = parseInt(zIndexOfElement) || 0;
			if (zIndexOfElementAsNumber > heighestZIndex) {
				heighestZIndex = zIndexOfElementAsNumber;
			}
		}
	});
	var currentZIndex = this.uiDialog.css('z-index');
	
	var moved;
	if (currentZIndex >= heighestZIndex) {
		moved = false;
	} else {
		this.uiDialog.css('z-index', heighestZIndex + 1);
		moved = true;
	}
	
	if ( moved && !silent ) {
		this._trigger( "focus", event );
	}
	
	return moved;
},
Last edited 10 years ago by cRz (previous) (diff)

comment:15 Changed 10 years ago by Jörn Zaefferer

Based on the comment from Bk1, apart from stacking, we also need to test scroll behaviour. I don't think we ever tested what happens when we move a dialog that has scrollable content.

comment:16 Changed 9 years ago by Jörn Zaefferer

After discussing that with Scott a while ago, there's a few things to do here:

  • Add unit tests for/against iframe and scroll reset.
  • If element isn't in front, look for. ui-front siblings, find highest z-index, increment by one, set to element.
  • Verify result with visual tests.

I still intend to work on this.

comment:17 Changed 9 years ago by Jörn Zaefferer

#9067 is a duplicate of this ticket.

comment:18 Changed 9 years ago by Jörn Zaefferer

Resolution: fixed
Status: openclosed

Dialog: Switch back to shuffling z-index, but only look at .ui-front siblings.

Fixes #9166 - Dialog: moveToTop implementation resets flash/video/iframe/scroll Fixes #9364 - Dialog: Click of element with tooltip scrolls the dialog to the top

Changeset: e263ebda99f3d414bae91a4a47e74a37ff93ba9c

comment:19 Changed 9 years ago by tvdeyen

Great, thanks!

comment:20 Changed 9 years ago by tj.vantoll

#9615 is a duplicate of this ticket.

comment:21 Changed 9 years ago by tj.vantoll

#9698 is a duplicate of this ticket.

comment:22 Changed 9 years ago by tj.vantoll

#9725 is a duplicate of this ticket.

comment:23 Changed 9 years ago by tj.vantoll

#10116 is a duplicate of this ticket.

Note: See TracTickets for help on using tickets.