optimize and clean up child view event handling code

RESOLVED FIXED

Status

()

RESOLVED FIXED
12 years ago
12 years ago

People

(Reporter: jaas, Assigned: jaas)

Tracking

Trunk
PowerPC
macOS
Points:
---

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

17.61 KB, patch
stuart.morgan+bugzilla
: review+
mikepinkerton
: superreview+
Details | Diff | Splinter Review
5.93 KB, patch
stuart.morgan+bugzilla
: review+
pavlov
: superreview+
Details | Diff | Splinter Review
(Assignee)

Description

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

Comment 1

12 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

12 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

12 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

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

Comment 5

12 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

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

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

Updated

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

Updated

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

Updated

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

Comment 7

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