optimize and clean up child view event handling code

RESOLVED FIXED

Status

()

Core
Widget: Cocoa
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Josh Aas, Assigned: Josh Aas)

Tracking

Trunk
PowerPC
Mac OS X
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

17.61 KB, patch
Stuart Morgan
: review+
Mike Pinkerton (not reading bugmail)
: superreview+
Details | Diff | Splinter Review
5.93 KB, patch
Stuart Morgan
: review+
Stuart Parmenter
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
There are a few simple things we can do to optimize and clean up child view event handling code.
(Assignee)

Comment 1

11 years ago
Created attachment 256596 [details] [diff] [review]
fix v1.0

1. All key events have 0,0 for a refpoint so just handle that in the key event conversion function instead of making callers do it every time they convert a key event.

2. Right now we ignore the message argument when creating gecko events, then we pass the message all the way through the conversion chain just to set it on the gecko event at the end. This is silly, just set the message when the event is created and stop passing it through the conversion chain as an unnecessary argument. Optimized signatures and simpler code. Win win.
Attachment #256596 - Flags: review?(stuart.morgan)

Comment 2

11 years ago
Comment on attachment 256596 [details] [diff] [review]
fix v1.0

Makes sense to me.

One other thing that would be nice to clean up:
convertLocation:... is a really inaccurate name; ideally it should just be folded into convertEvent:toGeckoEvent: but that messes up the drag stuff. Maybe split it into a few methods with accurate names, have convertEvent:... call all of them, and have the drag code call just the ones it needs?
Attachment #256596 - Flags: review?(stuart.morgan) → review+
(Assignee)

Updated

11 years ago
Attachment #256596 - Flags: superreview?(mikepinkerton)
Comment on attachment 256596 [details] [diff] [review]
fix v1.0

sr=pink
Attachment #256596 - Flags: superreview?(mikepinkerton) → superreview+
(Assignee)

Comment 4

11 years ago
landed on trunk, probably coming up with another patch so not marking as fixed
(Assignee)

Comment 5

11 years ago
Created attachment 256650 [details] [diff] [review]
convertLocation cleanup, v1.0

Get rid of convertLocation... The name didn't make much sense, the code was messy, and it was inefficient.
Attachment #256650 - Flags: review?(stuart.morgan)

Comment 6

11 years ago
Comment on attachment 256650 [details] [diff] [review]
convertLocation cleanup, v1.0

r=me
Attachment #256650 - Flags: review?(stuart.morgan) → review+
(Assignee)

Updated

11 years ago
Attachment #256650 - Flags: superreview?(mikepinkerton)
(Assignee)

Updated

11 years ago
Attachment #256650 - Flags: superreview?(mikepinkerton) → superreview?(pavlov)

Updated

11 years ago
Attachment #256650 - Flags: superreview?(pavlov) → superreview+
(Assignee)

Comment 7

11 years ago
convertLocation fix landed on trunk
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.