Overlay is re-applying behaviors in overlay.bindChild() just for the tabs. There is no need for this (there was before the clickHandler, but not any more) and it also causes overlay to break very easily in combination with other scripts using onhashchange handlers (e.g. #546126: Toolbar interferes with anchor links).

I'd say: remove this.

CommentFileSizeAuthor
#5 overlaybehaviors.patch1.14 KBcasey
overlaybehaviorstwice.patch779 bytescasey
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented

I don't know how to test this. I definitly fixes the issue i had when applying #546126: Toolbar interferes with anchor links.

Status: Needs review » Needs work

The last submitted patch, overlaybehaviorstwice.patch, failed testing.

Status: Needs work » Needs review

Re-test of overlaybehaviorstwice.patch from comment @comment was requested by aspilicious.

sun’s picture

sun
Primary language German
Location Karlsruhe
CreditAttribution: sun commented
Status: Needs review » Needs work 
+++ modules/overlay/overlay-parent.js	14 Jan 2010 12:27:02 -0000
@@ -441,10 +453,6 @@
     $tabs = $(self.$iframeWindow('<div>').append($tabs).remove().html());
 
     self.$dialogTitlebar.append($tabs);
-    if ($tabs.is('.primary')) {
-      $tabs.find('a').removeClass('overlay-processed');
-      Drupal.attachBehaviors($tabs);
-    }

.html() removes the bound events.

You either want to use .clone() to duplicate the tabs, or you want to properly use Drupal.detachBehaviors() along with .removeOnce(), so you can properly invoke Drupal.attachBehaviors() on the duplicated tabs again.

Powered by Dreditor.

casey’s picture

casey CreditAttribution: casey commented
Status: Needs work » Needs review
FileSize
overlaybehaviors.patch1.14 KB

Ok you're right.

    // Move the tabs markup to the title section. We need to copy markup
    // instead of moving the DOM element, because Webkit and IE browsers will
    // not move DOM elements between two DOM documents.
    $tabs = $(self.$iframeWindow('<div>').append($tabs).remove().html());

We can't use clone so we need Drupal.attachBehaviors().

Patch removes cruft (overlay doesn't apply behaviors to links any more) and also re-applies behaviors to the also copied shortcut link.

sun.core’s picture

sun.core CreditAttribution: sun.core commented
Priority: Critical » Normal

Not critical. Please read and understand Priority levels of Issues, thanks.

However, this patch looks RTBC if it still applies.

sun.core’s picture

sun.core CreditAttribution: sun.core commented

#5: overlaybehaviors.patch queued for re-testing.

aspilicious’s picture

aspilicious CreditAttribution: aspilicious commented
Status: Needs review » Reviewed & tested by the community

Applies!

==> RTBC per #6

Dries’s picture

Dries CreditAttribution: Dries commented
Status: Reviewed & tested by the community » Fixed

Committed to CVS HEAD. Thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.