Closed Bug 139877 Opened 22 years ago Closed 22 years ago

right-click context menu access keys require "enter" to perform action

Categories

(SeaMonkey :: UI Design, defect)

Product:
SeaMonkey
Component:
UI Design
Platform:
x86
All
Type:
defect
Priority:
Not set
Severity:
major

Tracking

(Not tracked)

Status:
VERIFIED FIXED

People

(Reporter: det, Assigned: yuanyi21)

References

Details

(Keywords: access, regression, Whiteboard: [driver:brendan] checked into trunk, seeing a=)

Attachments

(4 files, 3 obsolete files)

more comments about popupset
1.01 KB, patch
aaronlev
: review+
Details | Diff | Splinter Review
another patch that added a new atom - nsXULAtoms::popupset
1.39 KB, patch
aaronlev
: review+
jst
: superreview+
Details | Diff | Splinter Review
patch for trunk
853 bytes, patch
zhayupeng
: review+
jag+mozilla
: superreview+
Details | Diff | Splinter Review
patch for 1.0 branch
783 bytes, patch
zhayupeng
: review+
jag+mozilla
: superreview+
asa
: approval+
Details | Diff | Splinter Review 
From Bugzilla Helper:
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:0.9.9+) Gecko/20020424
BuildID:    2002042412

right-click anywhere
hit the hot-key for the menu item
the cursor jumps to the appropriate menu list entry
no action occurs.
hit enter, then action.
such as: right-click, 'b' for Back, nothing. with cursor still on 'Back' hit
"Enter" key.  browser goes back 1 page.  same for other entries.

Worked as desired in 20021814.  Broken in 2002042412.

Reproducible: Always
Steps to Reproduce:
1.right-click
2.hit 'b' for Back
then nothing. 

3. with cursor still on 'Back' hit "Enter" key.
browser goes back 1 page.
same for other menu entries.


Actual Results:  action not performed until "enter" key was pressed.

Expected Results:  menu action should happen when the hotkey is pressed.

works fine in 2002041814. failes in 2002042412.
I had the wrong build ID's: 


works in 2002041814
fails in 2002042412

also, in 2002042412, if i right click, and hit multiple hotkeys, the menu cursor
does not jump to menu entry after the first one.

right click, hit 'b' for 'Back'.  then hit 'r' for 'Reload'.  cursor does not
advance to "Reload" entry.

to XP apps.
Assignee: attinasi → sgehani
Status: UNCONFIRMED → NEW
Component: Layout → XP Apps
Ever confirmed: true
QA Contact: petersen → paw
another update

this bug is also not present in 20021903 (win32).
hope that helps narrow the scope a bit.

argh.. 2002041903 (win32, windows2000).. damnit i swear i am either blind or my
fingers are broken!
sorry :(
-> Blake for a look
Assignee: sgehani → blaker
qa --> sairuh@netscape.com
QA Contact: paw → sairuh
yep, i see this both in trunk and branch builds on both win2k and linux rh7.2.

this was working in 4/22 builds...will narrow down some more...
Keywords: access, nsbeta1, regression
OS: Linux → All
Summary: right-click context menu hotkeys require "enter" to perform action → right-click context menu access keys require "enter" to perform action
looks like this broke sometime btwn the 2002.04.22.08 and 2002.04.22.21 builds.
It maybe my fault. Taking...
Assignee: blaker → kyle.yuan
fixed
Attachment #81098 - Attachment is obsolete: true
Status: NEW → ASSIGNED
you guys are awesome.  will this be in the next round of nightly builds?  i'll
grab a new build at work in the morning and let you know what i find.

open source kicks.  you guys rock!  thanks a gig!

previous patch is wrong
Attachment #81111 - Attachment is obsolete: true
Comment on attachment 81122 [details] [diff] [review]
treat right-click menu as menu in FindMenuWithShortcut()

I have tested, it works fine.
r=pete.zha@sun.com
Attachment #81122 - Flags: review+
Duane its not on the trunk yet.. still need sr and that and checkin so until you
see that here.. dont expect it to be in the tree for you to test.. with a
downloadable build.  You can test the patch of course if you build mozilla
yourself. :)
*** Bug 140650 has been marked as a duplicate of this bug. ***
add some comments to explain why we usetagName.EqualsIgnoreCase("popupset") 
instead of nsXULAtoms::popupset
Attachment #81122 - Attachment is obsolete: true
Comment on attachment 81629 [details] [diff] [review]
more comments about popupset 

r=aaronl on comment addition
Attachment #81629 - Flags: review+
Should this patch be adding nsXULAtoms::popupset rather than get around the fact
that it's missing?
*** Bug 141258 has been marked as a duplicate of this bug. ***
Keywords: mozilla1.0
I don't know whether there is potential risk with the new atom. But it make the
code more clear.
Comment on attachment 82781 [details] [diff] [review]
another patch that added a new atom - nsXULAtoms::popupset

Kyle says he has tested this new patch, and it also works. I think adding the
XUL atom is the right fix.

r=aaronl
Attachment #82781 - Flags: review+
Who should super-review?  This bug is driving me crazy (I'm running a trunk
build).  As this is broken on the branch per sairuh (comment #7), we need an a=
from drivers for 1.0 after the sr= and the trunk landing.

/be
Blocks: 138000
Whiteboard: [driver:brendan]
Comment on attachment 82781 [details] [diff] [review]
another patch that added a new atom - nsXULAtoms::popupset

sr=jst
Attachment #82781 - Flags: superreview+
Whiteboard: [driver:brendan] → [driver:brendan] checked into trunk, seeing a=
I applied the patch and it worked for B in the navigator context menu, but I
still can't delete spam without reading it using right-click, d -- I must then
hit Enter.  Do we need some kind of mailnews patch too?

/be
Sorry, I always forgot the mailnews.....
New patch is coming...
Attached patch patch for trunk — — Splinter Review 

    
    

    
  


  
treat everything as menu except menulist.

    
    

    
  
In this bug, I have two ways to judge whether a MenuPopupFrame is a real menu.
1) if (tag == nsXULAtoms::menu || tag == nsXULAtoms::toolbarbutton || tag == 
nsXULAtoms::popupset || tag == nsXULAtoms::window)
   "menu" is the popup menu in menubar,
   "toolbarbutton" is the popup menu in toolbar
   "popupset" is the popup menu in navigator
   "window" is the popup menu in mailnews
   (I don't know whether popup menu has another name in other place)
2) if (tag != nsXULAtoms::menulist)
   treat everything as menu, except menulist (combobox drop-down menu)

I think the 2nd way is better, and make the patch by that way. 

jag, Could you r= for the last two patches?

    
    

    
  
|tag == nsXULAtoms::window|

It seems wrong to me that the tag has that name in Mail/News (can anyone here
explain that to me?). Regardless, your approach to check for |tag !=
nsXULAtoms::menulist| seems okay, see for example line 229 in the same file.
r=/sr=jag

    
    

    
  
I use those code in nsMenuPopupFrame::FindMenuWithShortcut.

  mContent->GetParent(*getter_AddRefs(parentContent));
  if (parentContent) {
    nsCOMPtr<nsIAtom> tag;
    parentContent->GetTag(*getter_AddRefs(tag));
    // I'm pretty sure tag == nsXULAtoms::window when it's popup menu in
    // mailnews window
    ......
  }

jag, If you think my patches are okay, could you mark them r=/sr= ?

    
    

    
  
Nevermind, I get it.

    
    

    
  
Comment on attachment 82990 [details] [diff] [review]
patch for trunk

sr=jag

        Attachment #82990 -
        Flags: superreview+

    
    

    
  
Comment on attachment 82993 [details] [diff] [review]
patch for 1.0 branch

sr=jag

        Attachment #82993 -
        Flags: superreview+

    
    

    
  
Comment on attachment 82993 [details] [diff] [review]
patch for 1.0 branch

r=pete.zha@sun.com

        Attachment #82993 -
        Flags: review+

    
    

    
  
Comment on attachment 82990 [details] [diff] [review]
patch for trunk

r=pete.zha@sun.com

        Attachment #82990 -
        Flags: review+

    
    

    
  
Comment on attachment 82993 [details] [diff] [review]
patch for 1.0 branch

a=asa (on behalf of drivers) for checkin to the 1.0 branch

        Attachment #82993 -
        Flags: approval+

    
    

    
  
fixed. checked into trunk and 1.0 branch
Status: ASSIGNED → RESOLVED
Closed: 22 years ago
Resolution: --- → FIXED

    
    

    
  
Kyle fixed this on the branch already, added fixed1.0.0
Keywords: fixed1.0.0

    
    

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

    
    

    
  
OK, tried it in win98 / build 2002051006 (aka RC2), works fine, no bug.
Somebody with proper authority, close it?

    
    

    
  
i tested build 2002051010 (linux x86) and build 2002051008 (win32 [win2k]) and
it's good.   i'm the original bug reporter, so do i close this or does the
assigned-to person? (i'm a bit new at this)

thanks guys! u all rock!!  no kidding...this is awesome stuff!


    
    

    
  
Took just slightly more than 2 weeks from bugreport to bugfix. And it cost only
$0.00.  Talk about Microsoft! ;)

    
    

    
  
Marking verified based on original reporter's comments in comment 42.  (Duane,
you can verify any bug you want to.)
Status: RESOLVED → VERIFIED

    
    

    
  
posthumus adt1.0.0+, pls get ADT approval prior to checking in to the 1.0 branch.
Keywords: adt1.0.0 → adt1.0.0+

    
    

    
  
verified on the branch using 2002.06.27.0x comm bits on linux rh7.2 and win2k.
Keywords: fixed1.0.0 → verified1.0.0

    
    

    
  
yeah this is long since been fixed and working. is this due to be marked
"CLOSED" then?

    
    

    
  
Duane, the CLOSED resolution is basically never used in Mozillas bugdatabase. It
serves no real purpose.
Product: Core → Mozilla Application Suite

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: