Closed Bug 338718 Opened 18 years ago Closed 18 years ago

open/close a popup-window using javascript does not work properly

Categories

(Core :: DOM: UI Events & Focus Handling, defect)

Product:
Core
Component:
DOM: UI Events & Focus Handling
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: volker.ertel, Assigned: roc)

References

Details

(Keywords: fixed1.8.1, testcase)

Attachments

(2 files)

Testcase from Comment #3
(deleted), text/html
Details
fix
(deleted), patch
MatsPalmgren_bugz
: review+
bzbarsky
: superreview+
MatsPalmgren_bugz
: approval-branch-1.8.1+
Details | Diff | Splinter Review 
User-Agent:       Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3
Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; de; rv:1.8.0.3) Gecko/20060426 Firefox/1.5.0.3

The following code works with IE6, but not with Firefox 1.5.03

<HTML><HEAD><TITLE>test</TITLE>
<SCRIPT language="JavaScript">
<!--
var box
function Zeige(Text)
  { box=window.open("","nbox","width=530,height=30,left=200") ; box.window.document.write(Text) ;
    box.window.document.bgColor="#fef6ec" ; box.window.document.close() ; box.window.focus() }
function Aus()
  {  box.window.close() }
  //-->
</SCRIPT>
</HEAD>
<BODY>
<br><br><br><br><br>
<A HREF="index.htm" onMouseOver="Zeige('Joachim')" onMouseOut="Aus()">Test</A>
</BODY></HTML>

Firefox opens several windows instead of only one as IE6 does.

Reproducible: Always

Actual Results:  
onMouseOver opens several boxes.
Internet Explorer 6 opens a sigle box.

Expected Results:  
onMouseOver should open a single box with text. onMouseOut should close this box.
IE6 works as expected.
I believe the code you have there is acting as it should.

Every time the mouse is moved over the link the "Zeige" function is called. Every time this happens a new window instance is created. Seems it should be opening multiple windows.
The event fires multiple times because the focus enters the new window and then the mouseover event is fired again.

If you use proper event handlers in your Javascript this can be avoided as you can remove the event handler after it is fired once and then attach it again after mouseout closes the window.

This solution works in both Firefox and Internet Exploter.

-- code: --

<HTML><HEAD><TITLE>test</TITLE>
<SCRIPT language="JavaScript">
//<!--
if (window.addEventListener)
  window.addEventListener('load', doOnload, false);
else if (window.attachEvent)
  window.attachEvent('onload', doOnload);

function doOnload()
{
  link = document.getElementById('test');

  if (link.addEventListener)
    link.addEventListener('mouseover', doLinkMouseOver, false);
  else if (link.attachEvent)
    link.attachEvent('onmouseover', doLinkMouseOver);
}

function doLinkMouseOver(e)
{
  link = e.target || e.srcElement;
  if (link.addEventListener) {
    link.addEventListener('mouseout', doLinkMouseOut, false);
    link.removeEventListener('mouseover', doLinkMouseOver, false);
  }
  else if (link.attachEvent) {
    link.attachEvent('onmouseout', doLinkMouseOut);
    link.detachEvent('onmouseover', doLinkMouseOver);
  }
  
  Zeige('Joachim');
}

function doLinkMouseOut(e)
{
  Aus();
  
  link = e.target || e.srcElement;
  if (link.addEventListener) {
    link.removeEventListener('mouseout', doLinkMouseOut, false);
    link.addEventListener('mouseover', doLinkMouseOver, false);
  }
  else if (link.attachEvent) {
    link.detachEvent('onmouseout', doLinkMouseOut);
    link.attachEvent('onmouseover', doLinkMouseOver);
  }
}

var box;

function Zeige(Text)
{
  box = window.open("","nbox","width=530,height=30,left=200");
  box.document.write(Text);
  box.document.bgColor="#fef6ec";
  box.document.close();
  box.focus();
}

function Aus()
{
  box.window.close();
  box = null;
}
//-->
</SCRIPT>
</HEAD>
<BODY>
<br><br><br><br><br>
<a id="test" href="index.htm">Test</a>
</BODY></HTML>
It is realy crazy. Even the simplest basic test does not work:

<HTML><HEAD><TITLE>test</TITLE>
<SCRIPT language="JavaScript">
<!--
var b;
function OpenBox()
  { b=window.open("","newbox","width=100,height=30") } 
function CloseBox()
  { b.close() }
  //-->
</SCRIPT>
</HEAD>
<BODY>
<br><br><br><br><br>
<A HREF="index.htm" onMouseOver="OpenBox()" onMouseOut="CloseBox()">Test</A>
</BODY></HTML>

When the mouse touches the link several (!) identical boxes open (instead of 
one). One of the boxes disappear when the mouse leaves the link. On the next
touch no additional boxes open. When leaving the link another box closes.
Some erratic number of identical boxes may appear during the course of the test.

I may have discovered an important basic bug if I am not mad.

(My intention was to provide links with additional extended comments similar to
the feature achieved with title="...".)

It looks like the onmouseover event is being fired multiple time - maybe the creation of a window is doing something that changes the cursors focus then switches it back after the creation causing the event to fire again.
Component: General → JavaScript Engine
Product: Firefox → Core
Version: unspecified → 1.0 Branch
> (My intention was to provide links with additional extended comments similar to
> the feature achieved with title="...".)

Don't use the creation of secondary windows to do that. Just use title="..." in the <a href="...">: you have a 90+ characters chunk in there. 
Creating, editing, focusing and closing windows by hovering/removing cursor is very resource-demanding, cpu and memory demanding and is utterly unreliable because ultimately all under the user preferences control/veto.

Window creation and loading+editing of referenced document Firefox is done asynchronously. 
Firefox users can determine which events can successfully create a secondary window (resorting to window.open()):
dom.popup_allowed_events
Firefox users can also prevent scripts from closing windows:
dom.allow_scripts_to_close_windows

Minimum height of script-initiated windows is 100px, not 30px.

My 2 cents
Attached file Testcase from Comment #3 (deleted) — 

    
    

    
  
-> Event Handling

The regression range is 2005-03-28-05 -- 2005-03-29-05:
http://bonsai.mozilla.org/cvsquery.cgi?treeid=default&module=all&branch=HEAD&branchtype=match&dir=&file=&filetype=match&who=&whotype=match&sortby=Date&hours=2&date=explicit&mindate=2005-03-28+04%3A00&maxdate=2005-03-29+06%3A00&cvsroot=%2Fcvsroot

In that span, bug 284664 seems the most likely to have caused it.
Assignee: nobody → events
Blocks: 284664
Component: JavaScript Engine → Event Handling
Keywords: testcase
OS: Windows XP → All
QA Contact: general → ian
Version: 1.0 Branch → Trunk

    
    

    
  

      
      
      
      

        Attached patch
          
          
        fix (deleted)
        — 
          — Splinter Review
      

    


  
To be honest, I don't know why this would have worked before. But it seems pretty clear that this "recursion check" is broken.
Assignee: events → roc
Status: UNCONFIRMED → ASSIGNED

        Attachment #224015 -
        Flags: review?(mats.palmgren)

    
    

    
  
Comment on attachment 224015 [details] [diff] [review]
fix

(In reply to comment #8)
> To be honest, I don't know why this would have worked before. But it seems
> pretty clear that this "recursion check" is broken.

I think so too.


> nsEventStateManager::NotifyMouseOver(nsGUIEvent* aEvent, nsIContent* aContent)
> {
>   NS_ASSERTION(aContent, "Mouse must be over something");
> 
>   if (mLastMouseOverElement == aContent)
>     return;
> 
>   // Before firing mouseover, check for recursion
>-  if (mLastMouseOverElement == mFirstMouseOverEventElement &&
>-      mFirstMouseOverEventElement)
>+  if (aContent == mFirstMouseOverEventElement && mFirstMouseOverEventElement)
>     return;

I don't think we need "&& mFirstMouseOverEventElement" since aContent
is always non-null here.

r=mats

        Attachment #224015 -
        Flags: review?(mats.palmgren) → review+

        Attachment #224015 -
        Flags: superreview+

    
    

    
  
checked in.
Status: ASSIGNED → RESOLVED
Closed: 18 years ago
Resolution: --- → FIXED

    
    

    
  
Comment on attachment 224015 [details] [diff] [review]
fix

I think we should fix this regression for FF2.

        Attachment #224015 -
        Flags: approval-branch-1.8.1?(mats.palmgren)

    
    

    
  
Comment on attachment 224015 [details] [diff] [review]
fix

This looks good also for the branch

        Attachment #224015 -
        Flags: approval-branch-1.8.1?(mats.palmgren) → approval-branch-1.8.1+

    
    

    
  
Fixed in 1.8 branch.
Keywords: fixed1.8.1
Component: Event Handling → User events and focus handling

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: