Closed Bug 30841 Opened 25 years ago Closed 23 years ago

Double right-clicking on a page disables keyboard

Categories

(Core :: XUL, defect, P4)

Product:
Core
Component:
XUL
Platform:
x86
Windows 98
Type:
defect

Tracking

()

Status:
VERIFIED FIXED
Milestone:
mozilla1.0

People

(Reporter: bugzilla, Assigned: dr)

References

(
URL
)

Details

(Keywords: helpwanted, Whiteboard: [br])

Attachments

(3 files)

...or it could be really easy.
658 bytes, patch
Details | Diff | Splinter Review
dean's sexy fix
685 bytes, patch
Details | Diff | Splinter Review
bah
971 bytes, patch
Details | Diff | Splinter Review 
If you double right-click (right-click two times fast after each other) the all 
input fields refuses to take any input.

To reproduce this weird bug:
- go to http://bonsai.mozilla.org/
- enter some text in one of the input fields.
- double right-click anywhere on the page

Now it's impossible to enter any text in any of the input fields.
This actually disables all input fields in the session. I had to restart Mozilla 
to be able to input text into fields again!
I did this and then started to get JS errors (dialog popups) and then it ended 
up crashing on me.
Assignee: rods → hyatt
Status: NEW → ASSIGNED
Target Milestone: M18
Mass moving M18 bugs to M19
Target Milestone: M18 → M19
mass-moving all bugs to m21 that are not dogfood+ or nsbeta2+ or nsbeta2-
Target Milestone: M19 → M21
I think this is kind of *very* bad....
Severity: normal → major
I don't see any errors or crashes, and it just disables text fields in the
current window, there is no need to restart.  Considering how unlikely a
double-right-click is, I'm inclined to future this, but first I'll reassign to
saari for a bit of investigation.
Henrik, why do you think this is so bad?
I don't see any errors or crashes, and it just disables text fields in the
current window, there is no need to restart.  Considering how unlikely a
double-right-click is, I'm inclined to future this, but first I'll reassign to
saari for a bit of investigation.
Henrik, why do you think this is so bad?
Assignee: hyatt → saari
Status: ASSIGNED → NEW
PDT: Nominating for nsbeta3+
Reason: This disables text fields in all windows now.  

Upgrading severity to critical and adding nsbeta3 keyword

Additional note:  When I try to type another url (bugzilla.mozilla.org) typing 
the 'b' activates the first item on the right-click popup menu, 'Back'...I tried 
typing some of the other hotkey'd items, and it seems as if the double-right 
click may be causing the popup window to be active (even though it is not seen), 
which also might explain why the text fields are not accessible...
Severity: major → critical
Keywords: nsbeta3
Is this related to bug 49897?  cc Pinkerton.  I still think double right-click
is such an unlikely scenario that this does not qualify for our short list. BTW
Chris, the PDT is not triaging nsbeta3 nominations, jrgm and I are.  cc'ing him.
Don't know if it is related or not, but it sounds pretty strange!
First guess is that the context menu's key listener is still active, but that
doesn't quite explain why typing fires the key-equivalents w/o having the
modifier keys be down.
Status: NEW → ASSIGNED
nsbeta3-, since it is such an unlikely scenario
Whiteboard: [nsbeta3-]
Updating QA contact.
QA Contact: ckritzer → vladimire
*** Bug 54764 has been marked as a duplicate of this bug. ***
Keywords: relnote3
Saari, you said:

"First guess is that the context menu's key listener is still active, but that
doesn't quite explain why typing fires the key-equivalents w/o having the
modifier keys be down."

Why doesn't that explain it?  It makes perfect sense to me.  If the key 
listener for the *context menu* is still active, a modifier doesn't need to be 
held down -- just a press of the accesskey should work fine (and does). And 
that's clearly what's happening here; I followed the original steps, pressed R, 
and then brought up a context menu after the page had reloaded -- lo and 
behold, the Reload menu item (whose accesskey is R) was preselected in the menu 
(as a result of bug 38425).

cc'ing dean, there's a clear description of the problem so maybe it shouldn't 
be too hard.  i'm looking into it..
URL: 38425
URL: 38425
Adding rtm keyword. This should definately be fixed in the release version. I 
personally double click with right button from time to time and I find it really 
annoying when the browser stops responding correctly to keyboard every time I do 
that.
Keywords: rtm
rtm-, this is just too unlikely.  adding helpwanted keyword, would consider a
one-line (or so) safe fix
Keywords: helpwanted
Whiteboard: [nsbeta3-] → [nsbeta3-][rtm-]
Target Milestone: M21 → Future
Keywords: relnote3 → relnote
Whiteboard: [nsbeta3-][rtm-] → [nsbeta3-][rtm-] relnote-user
Not only does this disable input in all text fields but also disables typing an
URL in the addressbar and scrolling with PgUp/PgDn, arrow up and arrow down
keys. My opinion is that a double right click can happen more than you think.

Oh, and this problem remains for the whole session, had to close moz and
re-launch it to make the problem disappear. Seen in BuildID 2000102404-Trunk on
Win98
Target Milestone: Future → mozilla0.9
*** Bug 63024 has been marked as a duplicate of this bug. ***
Keywords: nsbeta3, rtm → nsbeta1
Whiteboard: [nsbeta3-][rtm-] relnote-user → relnote-user
Adding my two cents here since I reported the duplicate bug 63024.  Yes, double 
right clicking can be a common occurence.  We have single right clicking to 
bring up the context menu and double left clicking to bring up other special 
actions.  So somebody using these features can easily get confused and 
accidentally do a double left click.  It happened to me often enough to report 
bug 63024 and from the comments in this report it has apparently happened to 
others as well.
What special features do we support by double left clicking? I'm not aware of
any... FWIW, I have yet to hit this bug using the product everyday, and I'm a
Mac user (we're all supposedly double click happy).
saari wrote:

   What special features do we support by double left clicking? I'm not
   aware of any.

Selection of course.  Double-clicking on a piece of text will cause the entire 
word to be selected.  And a new feature was recently added whereby 
double-clicking on a blank text field will cause it to be prefilled if there 
is any saved wallet data for that field.
Okay, so I double click to select text all the time and I've never hit this. And
yes, I use 3 button mice on my Macs too. Anyway, yes, it should be fixed.
Actually, I think the summary should be changed to "Double right-clicking on a 
page disables keyboard".  I duplicated the steps and not only could I not type 
in input fields, but keyboard shortcuts didn't work (eg. Ctrl+F) and I couldn't 
tab between fields.  If someone else can confirm this, we should change the 
summary.
oh yeah, that's something end users will discover.
*** Bug 64347 has been marked as a duplicate of this bug. ***
Summary: Double right-clicking on a page disables input fields → Double right-clicking on a page disables keyboard
Re-summarizing.

Suggest upping the priority to P2.  It disables the keyboard functionality for 
the current window, but I don't think that users will realize that opening a new 
browser window will result in keyboard working for that new window.  Also, I'm 
worried that this may be due to a deeper problem with events getting confused.
Looking at saari's comments from August and blake's from september, coupled with 
personal experience, the first occurrence of the context menu isn't being 
destroyed before the second is created.  Where to start?... where to start?...
This fix seems to resolve it for me.  Can someone confirm/deny?

Also, although this was reported under Windows, it looks like it may have been 
cross-platform.  Has anyone tried to duplicate this bug on Windows or Mac?
Yes, the patch fixes it for me.
r=morse
Neat, I love patches like that. Just make sure to test it with tooltips, top
level menus, and context menus. Make sure there arn't any weird interactions
between them (shouldn't be, just being safe).
saari and i talked about it this morning (well, i did most of the talking) and
here's what we (I) think:

this patch addresses the effect, but not the cause. For some reason, sometimes,
we're dropping a listener on the floor and it's biting us hard. I don't think
this patch 100% fixes the problem as calling ClosePopup() just closes the popup
for the current popupset. If (and this is a bug, unknown if) the dbl-click
causes two different context menus to come up (maybe ender and navigator?), the
first one still won't get the proper close and we'll still be left with a rogue
listener eating all the keystrokes in the app.

This patch rocks, and it fixes probably 99% of the problems, but if we have the
time, I'd prefer to find the real root of this problem. Dean? Do you mind
digging around a little more and find why we're leaking the listener? *ducking*
Oh great, listeners.  What I'm best at.  (sarcasm)

I can do some digging around, sure.  My first idea, before I started looking at
the code, was that the second popup was being created before the first was
destroyed, thus my mention of events getting confused.  I'll start going down
that line first.
* throws his brand new Furby Baby at pinkerton *
OK, here's what looks to be happening.  When you pop up a context menu and then 
right-click once somewhere else on the window, a bunch of WM_MOUSEACTIVATE 
messages fly around and eventually the window receives a WM_RBUTTONDOWN message. 
 So eventually we get to the "else" at:

  http://lxr.mozilla.org/seamonkey/source/widget/src/windows/nsWindow.cpp#884

which calls nsMenuDismissalListener::Rollup().  But it looks like a double-right 
click on the page is transmitted in the form of a WM_MOUSEACTIVATE, so that "if" 
statement reaches the "else" portion, and the nsMenuDismissalListener::Rollup() 
is never called.

And here's the really silly part.  You know how I have this fixed in my tree 
right now?  I changed line 884 from "else if..." to simply "if...".  And it's 
working!  All these hours spent debugging this thing and I removed one word.

Please, someone, try my fix and tell me that it's more complicated than that!

Or something like that.  It's getting pretty late.... err... early.
Whoops... WM_MOUSEACTIVATE, so that "if" statement *never* reaches the "else" 
portion
Dean, great work, that makes perfect sense. Since context menus are top level
entities as far as the OS is concerned, we're going to get a flurry of
activates/deactivates for the parent window and each context menu. I'll leave it
to Pinkerton to review the patch, but this sounds spot on.
this does sound spot on, just test it _VERY_ well with popups like toolips, and
our favorite, the mail-compose auto-complete address widget.

once it passes that barrage of tests, r=pinkerton, and now i owe you two beers
(at least).
cc'ing hyatt to sr
This hasn't screwed anything up for me yet, including such things as:
- double right-clicking, once on the page then one on an input field (eg. in the
  Additional Comments field in this report and then on the page right above it)
- double right-clicking, once in an input field and once on a page
- the auto-complete widget off of the location field still (seems to) work
  as expected
- doesn't screw up my patch to bug 50798 (x-mouse and menus)
- tooltips still go away when expected (mouse move, mouse down, etc)
- the mail compose address auto-complete seems to work, although I've never
  really used it before
Blocks: 64849
But I'm hoping there will be more testing than just mine, which I did over a
whole five minutes.

Also, a side effect of fixing this bug is that it also fixes bug 64849, which
makes sense as well.  This backs up my thinking that this is the way to fix this
one.
For what it's worth, when I right click multiple times in rapid succession 
around a page (with this patch applied), a context menu fails to appear many 
times.  I don't see that without the patch applied (context menus always 
appear).
Hrm... I always get a context menu.  Blake, based on this and our differences 
on bug 64849, I wonder if one of us doesn't have something "extra" in their 
tree.
Idea: Blake, have you applied my patch for bug 50798?  I don't think this should 
make a difference, but maybe...

Also note that if you're doing a lot of right-clicking on a page, it can take a 
little bit of time for the final context menu to come up, especially on a debug 
build.
Nope, the patch for bug 50798 definitely isn't needed.  I just re-pulled 
everything and built from scratch.  I then confirmed that the bug existed as 
reported.  After changing the "else if" to an "if" as I suggest, the symptoms 
are definitely gone.

Blake, if I click around the window about 30 times, it does take about five 
seconds for the context menu to appear on my PII-350.  But it does eventually 
appear.  And the keyboard still works properly after dismissing it.

Win2000 sp1
Blake, Dean, Pink, did the fix for this ever get checked in?
Not according to lxr ...
URL: http://lxr.mozilla.org/seamonkey/sour...
And not to my knowledge...
Nope.
Sending this to the dr for checkup and checkin.
Assignee: saari → dr
Status: ASSIGNED → NEW
Attached patch dean's sexy fix — — Splinter Review 

    
  
Status: NEW → ASSIGNED

    
    

    
  
this patch is a simple concretization of what dean described. let's see if i can
squeeze this into moz0.8. looking for sr...
Target Milestone: mozilla0.9 → mozilla0.8

    
    

    
  
sr=hyatt


    
    

    
  
a=blizzard

    
    

    
  
checked in (hyatt confirmed the if vs. else-if was a logic error). cvs rev.
3.316. thanks dean!
Status: ASSIGNED → RESOLVED
Closed: 24 years ago
Resolution: --- → FIXED

    
  
Blocks: 68164

    
    

    
  
Reopening, due to bug 68164.  Hyatt, any ideas, since you thought my logic was
right?

(There goes my perfect non-regressing patch record...)
No longer blocks: 68164
Status: RESOLVED → REOPENED
Resolution: FIXED → ---

    
    

    
  
d'oh!
Severity: critical → normal

    
    

    
  
i'm having a pretty hard time figuring out the relationship between this bug,
its fix, and 68164... i was hoping for the quick kill, but i've got to push it
back to 0.9.

changing component to xptoolkit/widgets. dean, hoping you can bang on this some...
URL: http://lxr.mozilla.org/seamonkey/sour... → http://lxr.mozilla.org/seamonkey/sour...
Component: Form Submission → XP Toolkit/Widgets
Priority: P3 → P2
QA Contact: vladimire → jrgm
Target Milestone: mozilla0.8 → mozilla0.9

    
    

    
  
*** Bug 68772 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 68772 has been marked as a duplicate of this bug. ***

    
    

    
  
Dean, do you think you could scope this out some more?
Assignee: dr → dean_tessman
Status: REOPENED → NEW

    
  
Blocks: keydead

    
    

    
  
I have an idea why my fix caused the regression in bug 68164.  The menu was 
being dismissed, but it was being displayed immediately after.  This is because 
the click on the menu bar when the menu is active triggers a WM_MOUSEACTIVATE 
and then a WM_LBUTTONDOWN.

With the previous "else if (rollup)" after "if (inMsg == WM_MOUSEACTIVATE)", the 
WM_MOUSEACTIVATE was essentially ignored and the menu wasn't rolled up until the 
WM_LBUTTONDOWN made it through.

By changing the "else if" to "if" as I had suggested, both the WM_MOUSEACTIVATE 
and WM_LBUTTONDOWN are processed.  The WM_MOUSEACTIVATE rolls up the menu, and 
the WM_LBUTTONDOWN pops it down again.

Here's one way to resolve this.  Change the "else if" to an "if" as the second 
patch does, and then change line 866 of nsWindow.cpp from:
  if (uMsg == WM_MOUSEMOVE)
to:
  if ( (uMsg == WM_MOUSEMOVE) || (uMsg == WM_LBUTTONDOWN) )

This seems to work well for me, but I'd be interested to have someone else try 
it out.  Of couse, if someone has a better way of doing this, or sees a hole in 
my logic, let me know.

...

But this would make the second event-specific processing inside of the 
WM_MOUSEACTIVATE processing.  I'm wondering if we should set up a mechanism so 
that if the WM_MOUSEACTIVATE is followed by a similar stand-alone event, we 
ignore the stand-alone event.  For example, if we get a WM_MOUSEACTIVATE with a 
WM_LBUTTONDOWN as the activation message, and then get a WM_LBUTTONDOWN after 
that, ignore the WM_LBUTTONDOWN.  I have no idea what this would do to existing 
code.  And this assumes that I'm not way off base here.

    
    

    
  
i'll try making another pass at this, now that we have the NS_CONTEXTMENU event
enabled.
Assignee: dean_tessman → dr

    
    

    
  
->moz0.9.1, per triage
Target Milestone: mozilla0.9 → mozilla0.9.1

    
  
Status: NEW → ASSIGNED

    
  
Priority: P2 → P4

    
  
Priority: P4 → P3
Target Milestone: mozilla0.9.1 → mozilla0.9.2

    
  
Keywords: nsCatFood

    
    

    
  
[spam] pushing off non-critical 0.9.2 bugs to 0.9.3. please take the helpwanted
keyword to heart if you'd like to see these fixed in 0.9.2!
Target Milestone: mozilla0.9.2 → mozilla0.9.3

    
    

    
  
Dan, did you ever do any more digging based on your 04-03 comment?

    
    

    
  
Dean: Afraid I didn't get the chance :( Would you like the bug back?

    
    

    
  
Ummm... not really!  But I'll take it so I look at it again.  If I can't find 
anything, guess who it's coming back to!
Assignee: dr → dean_tessman
Status: ASSIGNED → NEW

    
    

    
  
I just read through this, and I didn't see this mentioned (though its certainly
possible I missed it):

When you double right-click under Windows XP (I haven't tested this with my
Windows Me installation), the right-click menu appears briefly and then
disappears.  When you press keys, the keyboard does not seem to work, generally,
however if you press V, the source window will appear, and if you press S, the
Save As dialog appears.

This would suggest (at least to me) that when the menu pops up and disappears,
the browser thinks the menu is still there, and is waiting for keyboard commands
that match the underlined letters in the menu (V for <u>V</u>iew Page Source,
etc)  I think this bug might be more related to the right-click menu
implementation rather than the mouse events specifically.

    
    

    
  
Yeah, that's what I meant by "the first occurrence of the context menu isn't 
being destroyed before the second is created."  :-)

I'm going to try going down two paths here.  The first is working off of my 
original patch and seeing why it caused bug 68164.  Anyone have any 
off-the-top-of-their-head ideas?

The second is to consume all WM_MOUSEACTIVATE messages.  That's a little dicey, 
but it's worth a shot based on my (apparent - can't quite remember them!) 
investigations on 03-18.

    
    

    
  
kevin, that's exactly the problem (sorry, we've known about it for a while).
there is a popup-menu listener registered and never unregistered and that's what
is eating all the keystrokes.

    
    

    
  
I saw a lot of pr1 complaints about this; Dean, any word?

    
    

    
  

      
      
      
      

        Attached patch
          
          
        bah
        — 
          — Splinter Review
      

    


  

    
    

    
  
This is just the "if ( (uMsg == WM_MOUSEMOVE) || (uMsg == WM_LBUTTONDOWN) )" 
idea that I mentioned in my 2001-03-18 babble.  It seems to work rather well for 
me.

I do notice, however, that if you click to dismiss a menu (on the menu bar) and 
then click to open it again that the menu title doesn't depress.  But that could 
be do to other things I've got going on in my tree.

    
    

    
  
*** Bug 84402 has been marked as a duplicate of this bug. ***

    
    

    
  
*** Bug 88012 has been marked as a duplicate of this bug. ***

    
    

    
  
These are my comments from my bug which I closed, that described the behavior
perfectly and sheds some more light on the problem:

Build 2001-06-26-11, Win98
Plus earlier builds (on Win2k) as well

Steps to reproduce:
Unknown. [Now confirmed to be at least Double-Right-Click on page]

Behavior:
When trying to type in an input box, there seems to be no response.  Pressing
other keys seems to ilicit responses, but not as expected.  I've mapped out the
entire keyboard and its responses:

V: Pulls up source of page
B: Goes back 1 page
F: Goes forward 1 page
R: Reloads current page
[S: Save as]
ENTER/KEYPAD ENTER: Goes back 1 page

No other keys perform any function.  F1-F12, ESC, 0-9, other keypad buttons,
tab, space...only the locks (i.e. caps, scroll, num) work (as they obviously
should.)

Also, I found two JS Error messages, but am only 80% sure they are related:

Error: redeclaration of const kIOServiceProgID
Source File: chrome://communicator/content/utilityOverlay.js
Line: 1

Error: contextMenu has no properties

Reproduceability:
Rare to occasional

Workaround:
Close all browser windows and reopen.  (I meant to close Mozilla altogether but
had a JS console open.  Opening a new browser window via CTRL+1 was enough to
stop the problem)


    
    

    
  
*** Bug 84402 has been marked as a duplicate of this bug. ***

    
    

    
  
I haven't made much progress on this, at least progress that I'm happy with.  
Back to you, dr.  You may want to check out my June 19 patch as a solution, if 
only a band-aid solution.
Assignee: dean_tessman → dr

    
  
Priority: P3 → --
Target Milestone: mozilla0.9.3 → ---

    
    

    
  
I'll give it a shot. IIRC, the |else if| -> |if| change wasn't correct, and
caused a regression which I can't remember right now... something like, you
couldn't dismiss menus or something. If the patch doesn't work, I'm not going to
have a clue what to do.
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.0

    
    

    
  
That's exactly what the problem was.  You could open a menu from the menu bar
fine, but clicking again wouldn't dismiss it.  That was bug 68164.

    
    

    
  
*** Bug 92148 has been marked as a duplicate of this bug. ***

    
  
Blocks: 94555

    
    

    
  
Pink, back to your 2001-01-05 comments.  This bug has 17 votes and about 10
duplicates, what do you think about using my fix which would address 99% of the
problems, but keeping this open to find the perfect solution?
No longer blocks: 94555

    
    

    
  
*** Bug 94555 has been marked as a duplicate of this bug. ***

    
  
Whiteboard: relnote-user → relnote-user [br]

    
    

    
  
hyatt somehow fixed this in his popup landing. three cheers for hyatt!

ok, one will do.
Status: ASSIGNED → RESOLVED
Closed: 24 years ago → 23 years ago
Resolution: --- → FIXED

    
    

    
  
Well I'll be a son of a...  I can't reproduce this anymore.  Verifying.
Status: RESOLVED → VERIFIED

    
    

    
  
Tricky... this works now because double-right-clicking doesn't dismiss the
context menu like it used to.

    
    

    
  
Removing relnote mentions.
Keywords: relnote
Whiteboard: relnote-user [br] → [br]

    
    

    
  
Hrm.  This still appeared in the 0.9.6 release notes, except referencing bug
70812 instead of this one.

    
    

    
  
Hrm.  This still appears in the 1.4RC1 release notes, except referencing bug
70812 instead of this one.

Somebody should tell Asa.

    
    

    
  
Asa, seems like this bug should be removed from the release notes (see comments
above).

    
    

    
  
I made a note of that in bug 207679.

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: