Consolidate NS_MOUSE_***_UP/DOWN/CLICK/DBLCLICK events

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
11 years ago

People

(Reporter: smaug, Assigned: smaug)

Tracking

Trunk
x86
All
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

to make nsEvent structures to work more like dom events,
nsIDOMMouseEvent::type should always map to only one nsMouseEvent::message.
I have a patch for this, but only for gtk2. Need to still convert other
toolkits.
Created attachment 242383 [details] [diff] [review]
for all toolkits

Need to still re-read the patch myself.
roc, jst, perhaps you have some comments to the changes in nsGUIEvent.h?
Looks good. it might be nice to just make the button number be an integer because mice can have many more buttons.
Looks fine to me too. We *could* save ourselves 32-bits per mouse event if we'd be ok with a 16 bit clickCount member, but either way is fine with me as these event objects don't tend to live very long...
Created attachment 244096 [details] [diff] [review]
for all toolkits 2

This is tested to work on linux/win/mac

I'm not quite sure how this should be reviewed.
Jst, perhaps you could check at least the content/ and layout/ and
say what you think.
Attachment #242383 - Attachment is obsolete: true
Attachment #244096 - Flags: review?(jst)
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

I looked through the content and layout parts and that looks all good to me. Maybe get the peers for the various widget libraries to look over the OS specific parts?
Attachment #244096 - Flags: review?(jst) → review+
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

josh, could you look at the mac code.
Attachment #244096 - Flags: review?(joshmoz)

Updated

12 years ago
Attachment #244096 - Flags: review?(joshmoz) → review+
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

Roc, could you look at the gtk/gtk2 part?
Attachment #244096 - Flags: review?(roc)

Comment 10

12 years ago
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

r=sergei_d
if number of arguments
here http://lxr.mozilla.org/seamonkey/source/widget/src/beos/nsWindow.cpp#3059 and here http://lxr.mozilla.org/seamonkey/source/widget/src/beos/nsWindow.cpp#3153 
will be chnaged from 5 to 6.
Attachment #244096 - Flags: review?(sergei_d) → review+
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

Ere, could you look at the Windows part?
Attachment #244096 - Flags: review?(emaijala)

Comment 12

12 years ago
This patch breaks BeOS mouse handling.  Left-click did not function even at startup profilemanager.  Partial console output of debug build:

CMA
TK-GI
###!!! ASSERTION: Wrong number of arguments to CallMethod: 'info->nArgs == 6', file /boot/home/develop/mozilla/widget/src/beos/nsWindow.cpp, line 1831
CMA
TK-GI
###!!! ASSERTION: Wrong number of arguments to CallMethod: 'info->nArgs == 6', file /boot/home/develop/mozilla/widget/src/beos/nsWindow.cpp, line 1831
CMA
TK-GI
###!!! ASSERTION: Wrong number of arguments to CallMethod: 'info->nArgs == 6', file /boot/home/develop/mozilla/widget/src/beos/nsWindow.cpp, line 1831
CMA
TK-GI
###!!! ASSERTION: Wrong number of arguments to CallMethod: 'info->nArgs == 6', file /boot/home/develop/mozilla/widget/src/beos/nsWindow.cpp, line 1831  

Comment 13

12 years ago
More specifically, clicking with left mouse button does not have intended result.  Example:  in profile manager, clicking left button on available profile, "start minefield" or "exit" button changes focus to the clicked item.  However, expected action (start or exit) never actually occurs, even with repeated clicking.  Also, assertion listed above disappears if "== 6" is replaced by "== 5".
Doug, did you read Sergei's review comments ;)

Comment 15

12 years ago
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

Windows part looks good to me.
Attachment #244096 - Flags: review?(emaijala) → review+
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

Michael, could you look at os2 part.
Attachment #244096 - Flags: review?(mozilla)
I built it into OS/2 and it seems to be working fine, is there something I should check in particular?  

Comment 18

12 years ago
Looks OK from an OS/2 specific.

I'm curious as to why a mouse button is passed along at all in the context menu key case?

Updated

12 years ago
Attachment #244096 - Flags: review?(mozilla) → review+
Checked in with some fixes for mac
Status: NEW → RESOLVED
Last Resolved: 12 years ago
Resolution: --- → FIXED
Mozilla/5.0 (Windows; U; Windows NT 5.0; en-US; rv:1.9a1) Gecko/20061120 Minefield/3.0a1 ID:2006112021 [cairo]

Is there any chance this has caused the context menu in both the add-ons manager and download manager to appear bottom left (regardless of pointer position) ?

Updated

12 years ago
Depends on: 361376

Comment 21

12 years ago
Comment on attachment 244096 [details] [diff] [review]
for all toolkits 2

>-    case NS_MOUSE_MIDDLE_BUTTON_UP:
>-    if(!gMiddlePref)
>+    case NS_MOUSE_BUTTON_UP:
>+      if (aEvent->eventStructType != NS_MOUSE_EVENT ||
>+          (NS_STATIC_CAST(nsMouseEvent*, aEvent)->button == nsMouseEvent::eMiddleButton &&
>+           !gMiddlePref)) {
>         break;
>+      }
> 
>-    case NS_MOUSE_LEFT_BUTTON_UP:
>-       // stop capturing
>-      //printf("stop capturing\n");
>-      AddListener();
>-      DragThumb(PR_FALSE);
>-      if (mChange) {
>-        nsRepeatService::GetInstance()->Stop();
>-        mChange = 0;
>+      if (NS_STATIC_CAST(nsMouseEvent*, aEvent)->button == nsMouseEvent::eLeftButton) {
>+         // stop capturing
>+        AddListener();
>+        DragThumb(PR_FALSE);
>+        if (mChange) {
>+          nsRepeatService::GetInstance()->Stop();
>+          mChange = 0;
>+        }
>+        mRedrawImmediate = PR_FALSE;//we MUST call nsFrame HandleEvent for mouse ups to maintain the selection state and capture state.
>+        return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus);
>       }
>-      mRedrawImmediate = PR_FALSE;//we MUST call nsFrame HandleEvent for mouse ups to maintain the selection state and capture state.
>-      return nsFrame::HandleEvent(aPresContext, aEvent, aEventStatus);
This change caused bug 364125 - the old code was equivalent to if ((button == 1 && gMiddlePref) || button == 0) but the new code doesn't do anything useful for middle buttons.
Depends on: 373483
You need to log in before you can comment on or make changes to this bug.