Opened 13 years ago

Closed 12 years ago

#1363 closed defect (fixed)

PopupManager misbehaves in iframes

Reported by: terry.field@… Owned by: bill
Priority: high Milestone: 0.9
Component: Widgets Version: 0.3
Keywords: Cc: terry.field@…, bill@…
Blocked By: Blocking:

Description

We've encountered a couple of problems with the PopupManager? in Menu2. These seem closely related to tickets #1230 and #1164.

Firstly, when you have Menu2 or some other widget that uses PopupManager? in an iframe, problems will arise when the iframe is closed. Our current workaround is to modify PopupManager? by adding:

this.unRegisterWin = function(win){
  if(win.__PopupManagerRegistered)
  {
    dojo.event.disconnect(win.document, 'onmousedown', this, 'onClick');
    dojo.event.disconnect(win, "onscroll", this, "onClick");
    dojo.event.disconnect(win.document, this._keyEventName, this, 'onKeyPress');
    win.__PopupManagerRegistered = false;
  }
};

this.unRegisterAllWindows = function(){
  for(var i=0;i<this.registeredWindows.length;++i){
    this.unRegisterWin(this.registeredWindows[i]);
  }
  this.registeredWindows = [];
};

dojo.addOnUnload(this, "unRegisterAllWindows");

The second problem is that a previously assigned non-dojo "onmousedown" in a parent frame will get hijacked. Our workaround is to modify the registerWin method:

  this.registerWin = function(win){
    if(!win.__PopupManagerRegistered)
    {
+
      var omd = win.document['onmousedown'];
      if (omd) {
        if((!dojo.lang.isFunction(omd))&&(!dojo.lang.isAlien(omd)))
          return;
      }
+
      dojo.event.connect(win.document, 'onmousedown', this, 'onClick');
      dojo.event.connect(win, "onscroll", this, "onClick");
      dojo.event.connect(win.document, this._keyEventName, this, 'onKeyPress');
      win.__PopupManagerRegistered = true;
      this.registeredWindows.push(win);
    }
  };

Attachments (1)

google-adwords-hack-fix.patch (650 bytes) - added by tberman@… 13 years ago.
hacky fix for google ads

Download all attachments as: .zip

Change History (13)

comment:1 Changed 13 years ago by bill

Cc: terry.field@… added
Owner: changed from bill to liucougar

Reassigning to liucougar. But before he can look at it, Terry, we need a CLA (www.dojotoolkit.org/icla.txt) from you.

comment:2 Changed 13 years ago by guest

This doesn't seem to fix ticket #1164. In my (breif) tests, there were no propblems when the page embedded an iframe of the same domain, but when using one for google adsense the "Permission denied to get property Window.PopupManagerRegistered" exception is not caught.

comment:3 Changed 13 years ago by tberman@…

We ran across the same issue, and added a horrible hacky fix for it, I will attach the patch.

Changed 13 years ago by tberman@…

hacky fix for google ads

comment:4 Changed 13 years ago by terry.field@…

I have signed the CLA and faxed it to you, as requested. (Terry Field)

comment:5 Changed 13 years ago by liucougar

could you tell me what's exact the first problem you mentioned?

About your proposed patch for your second problem, I am afraid it is inappropriate to be included in dojo. Please consider altering your code to make use of dojo event instead

CLA is confirmed on file

comment:6 Changed 13 years ago by guest

The first problem goes something like this:

You have a page that doesn't reference any dojo code. Within that page you dynamically create an iframe that includes dojo widgets, including Menu2 (or anything else that uses PopupManager?). Sometime later you close the iframe. Click anywhere on the original page and bang - you get an error because you still have a mousedown event but it points to non-existent code (ie code that was in the iframe that doesn't exist anymore).

I think you may be missing the point about the second problem. Let me descibe that scenario a little better. Like the first problem, you have a page without any dojo code. However, in this case there is a mousedown event being registered to some other javascript code (in our case, a third party menu system). When you dynamically create an iframe with dojo PopupManager? code embedded, it will hijack the mousedown event in the original page. My suggestion simply jumps over any instance where a mousedown event is already registered.

Perhaps an alternative would be to be to provide the ability to configure the PopupManager? so that it doesn't look outside its own frame/window. I guess this is really the crux of the problem since the PopupManager? probably shouldn't be interfering with non-dojo frames.

comment:7 Changed 13 years ago by liucougar

Cc: bill@… added

right, I will fix the first issue

about the second one, I'd like to hear Bill's opinion first, CCed

comment:8 Changed 13 years ago by liucougar

first issue is fixed in r5560

comment:9 Changed 13 years ago by liucougar

Milestone: 0.40.5

after talking with bill, the fix for second issue is not appropriate,

"in the long term somebody (alex?) should fix the event stuff so it works in conjunction w/this menu system. And I guess we need to get a testcase from them." (from Bill)

so please consider submiting a test case for your problem

comment:10 Changed 13 years ago by dylan

Component: WidgetsEditor

comment:11 Changed 13 years ago by dylan

Component: EditorWidgets
Owner: changed from liucougar to bill

oops, not an editor bug.

comment:12 Changed 12 years ago by bill

Resolution: fixed
Status: newclosed

The PopupManager? has been rewritten for 0.9 and I think/hope this problem is fixed. If not, please open a new bug with a testcase. Thanks!

Note: See TracTickets for help on using tickets.