Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 5695 Context menu for title bar doesn't appear when a tab's context menu was open
Starred by 3 users Reported by tobias...@gmail.com, Dec 18 2008 Back to list
Status: Verified
Owner:
Closed: Feb 2009
Cc:
EstimatedDays: ----
NextAction: ----
OS: All
Pri: 3
Type: Bug

Restricted
  • Only users with Commit permission may comment.



Sign in to add a comment
Chrome Version       : 1.0.154.39

What steps will reproduce the problem?
1. Open the context menu for a tab of your choice
2. Right-click on the title bar

What is the expected result?
The application context menu should show up.

What happens instead?
Nothing (The tab's context menu is correctly closed, of course).
 
Labels: -Area-Misc Area-BrowserUI
Status: Untriaged
Only happens on release build. Keeping it open only to make sure that the fix is
migrated over to release branch from trunk.
Labels: -Pri-2 Pri-3 Mstone-X HelpWanted
Status: Available
I'm actually seeing this same behavior in the ToT release.  This would probably be a 
good starter bug for someone, marking HelpWanted.
Comment 3 Deleted
Comment 4 Deleted
Comment 5 Deleted
Comment 6 Deleted
Comment 7 Deleted
Comment 8 Deleted
Comment 9 Deleted
Comment 10 Deleted
Comment 11 by Deleted ...@, Jan 8 2009
I think chrome browser lacks consistency in displaying context menu.


* Current State
---------------------------------------------------------------------
 context menu for the 'Tab' : Show up on right mouse button 'Release'
 context menu for the 'App' : Show up on right mouse button 'Press'
---------------------------------------------------------------------

This inconsistent concept (one by 'Release' and the other by 'Press')
is not natural (in the UI point of view) and can cause problems.


Currently, when the right mouse button is 'Pressed' on the title bar,
its handler seems to take following two steps in order.

1. Close the current context menu if it exists.
2. Create new context menu.

Processing two operations (Closing and Creating) at one time ('Press')
can cause conflict problems.

- If the current context menu is for an 'App', there's no problem. But
   if it is for a 'Tab', it seems that above two steps come into the 
   conflict and doesn't work well.


*****Suggestion *****

So, Divide two operations (Closing and Creating) into seperate handlers.
- Closing always on 'Press' and Creating always on 'Release'.

If this concept is applied, no conflict can occur.
- When the  mouse button is 'Pressed', current (for a Tab or for an App.)
  context menu is closed and when the button is 'Released', new context
  menu is created.

And, it is natural that displaying context menu is always done by 'Release'
operation. (MS IE also uses this concept.)


* Suggested State:
---------------------------------------------------------------------
 context menu for the 'Tab' : Show up on right mouse button 'Release'
 context menu for the 'App' : Show up on right mouse button 'Release'
---------------------------------------------------------------------


I wanna know your opinions about this.


Sounds reasonable to me... :)
That sounds reasonable to me as well, but concerning the main bug, any tips on how to
pass the events to the underlying component. I will try to look at it when I get home
tonight.
Comment 14 Deleted
Comment 15 by Deleted ...@, Jan 12 2009
While looking at the related code, I found the wrong code that lead to the problem. 
So, It would be better to fix this bug first and discuss about the suggestion later.

I made the patch and requested review.
- http://codereview.chromium.org/17608


Comment 16 Deleted
Comment 17 by Deleted ...@, Jan 15 2009
Currently, displaying context menu is conducted by RunSystemMenu() on 
Window::OnNCRButtonDown(). So, context menu is created on 'Press'.

I have changed the code that RunSystemMenu() is called not on OnNCRButtonDown() but 
on OnRButtonUp() with some modifications.

I have made my local patch and tested several times. And I think it works well.

As soon as the above bug fix is finished, I'll request review with this patch.
Comment 18 by Deleted ...@, Jan 19 2009
Patch on above suggestion:
http://codereview.chromium.org/18095
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=9761 

------------------------------------------------------------------------
r9761 | finnur@google.com | 2009-02-13 09:45:37 -0800 (Fri, 13 Feb 2009) | 65 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/views/chrome_menu.cc?r1=9761&r2=9760

A patch from external contributor Yong Shin (already in AUTHORS list). I also removed some lint errors. This was reviewed by me and Scott

TBR=sky

Original changelist description (http://codereview.chromium.org/17608):

Context menu for title bar doesn't appear when a tab's context menu was open.

---------------------------------------------------------------------------
* About the Bug
---------------------------------------------------------------------------

- What steps will reproduce the problem?

1. Open the context menu for a tab of your choice.
2. Right-click on the title bar.

- What is the expected result?

The application context menu should show up.

- What happens instead?

Nothing (The tab's context menu is correctly closed, of course).

- What is the reason?

The context menu for an App is displayed when WM_NCRBUTTONDOWN event is
processed on the Window::OnNCRButtonDown() handler.

In normal case, when the right mouse button is clicked on the NC area, this
event is generated. But in the case that the context menu for a tab is being
displayed,  WM_NCRBUTTONDOWN is not received. Instead, WM_NCLBUTTONDOWN is
generated. (This is wrong because left mouse button is never pressed.)

So the right handler (Window::OnNCRButtonDown()) cannot be called.

---------------------------------------------------------------------------
* About the Fix
---------------------------------------------------------------------------

What is modified?

Modified MenuController::RepostEvent() method. This function is called when the
mouse button is pressed while the context menu for a tab is being displayed.

In this function, following two steps occur in order.

1st. Determine the event type.
2nd. Post this event using PostMessage().

On the 2nd step,

In the case that the determined event on the 1st step is for the client area,
there's no problem. But in the case of the non-client area, it doesn't use the
right event type but use the fixed event type (WM_NCLBUTTONDOWN). This is the
wrong implementation.

So, I modified this part so that it use the right event type.


Bug= 5695 

 http://crbug.com/5695 
Review URL: http://codereview.chromium.org/21354
------------------------------------------------------------------------

Status: Fixed
Fixed at revision 9761.
Thanks, Yong!
Status: Verified
Build: 2.0.164.0 (Official Build 9941)
 Issue 9403  has been merged into this issue.
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=18170 

------------------------------------------------------------------------
r18170 | pkasting@chromium.org | 2009-06-11 10:21:35 -0700 (Thu, 11 Jun 2009) | 4 lines
Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/views/window/window_win.cc?r1=18170&r2=18169
   M http://src.chromium.org/viewvc/chrome/trunk/src/views/window/window_win.h?r1=18170&r2=18169

Show caption area context menu on mouseup, not mousedown.  Original patch by Yong Shin (see http://codereview.chromium.org/18095 ), r=me, modified by me.

BUG= 5695 
TEST=Right-click the titlebar area; the menu should not appear until mouse up.
------------------------------------------------------------------------

Project Member Comment 24 by bugdroid1@chromium.org, Oct 12 2012
Labels: Restrict-AddIssueComment-Commit
This issue has been closed for some time. No one will pay attention to new comments.
If you are seeing this bug or have new data, please click New Issue to start a new bug.
Sign in to add a comment