Bug 33783

Summary: [DRT][Mac] Add modifiers parameter to mouseDown() and mouseUp()
Product: WebKit Reporter: Kent Tamura <tkent>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, eric, sky
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: Macintosh   
OS: OS X 10.5   
Bug Depends on:    
Bug Blocks: 24161    
Attachments:
Description Flags
Patch
none
Proposed patch (rev.2) none

Description Kent Tamura 2010-01-17 22:55:23 PST
Add support for mouse events with key modifiers like:
  eventSender.mouseDown(0, ["addSelectionKey"])
  eventSender.mouseUp(0, ["shiftKey", "ctrlKey"])
Comment 1 Kent Tamura 2010-01-17 23:15:31 PST
Created attachment 46790 [details]
Patch
Comment 2 Darin Adler 2010-01-18 10:49:08 PST
Comment on attachment 46790 [details]
Patch

> -    [self performSelector:@selector(mouseDown:) withObject:nil afterDelay:0];
> -    [self performSelector:@selector(mouseUp:) withObject:nil afterDelay:0];
> +    [self performSelector:@selector(mouseDown:withModifiers:) withObject:nil afterDelay:0];
> +    [self performSelector:@selector(mouseUp:withModifiers:) withObject:nil afterDelay:0];

This isn't reliable. The argument to "withModifiers" will be random data on the stack. The object "nil" is passed as the first argument, but no second argument is passed. The simplest way I can think of to make this work is to keep a method with the old name and have it call the new method. There may be other alternatives.

review- because of this
Comment 3 Kent Tamura 2010-01-18 18:37:23 PST
Created attachment 46876 [details]
Proposed patch (rev.2)
Comment 4 Kent Tamura 2010-01-18 18:38:34 PST
(In reply to comment #2)
> This isn't reliable. The argument to "withModifiers" will be random data on the
> stack. The object "nil" is passed as the first argument, but no second argument
> is passed. The simplest way I can think of to make this work is to keep a
> method with the old name and have it call the new method. There may be other
> alternatives.

Thank you for the advice.  I did so in the updated patch.
Comment 5 Darin Adler 2010-01-19 08:26:35 PST
Comment on attachment 46876 [details]
Proposed patch (rev.2)

Looks fine.

r=me

Not directly related to this patch: I realize now that the use of performSelector:withObject: is already wrong for mouseDown: and mouseUp: since they have an argument that is not an object. Someone should clean this up in the future. I think we can just replace mouseDown: and mouseUp: with mouseDownButton0 and mouseUpButton0 methods that don't take arguments at all.
Comment 6 WebKit Commit Bot 2010-01-19 14:50:57 PST
Comment on attachment 46876 [details]
Proposed patch (rev.2)

Clearing flags on attachment: 46876

Committed r53498: <http://trac.webkit.org/changeset/53498>
Comment 7 WebKit Commit Bot 2010-01-19 14:51:04 PST
All reviewed patches have been landed.  Closing bug.