425526 - [NSEvent locationInWindow] used incorrectly in Cocoa widgets

Status: NEW

(Core :: Widget: Cocoa, defect, P4)

Description

[Steven Michaud [:smichaud] (Retired)]

[NSEvent locationInWindow] is used in several places in Cocoa widgets
to return the current location of the mouse in an NSEvent object's
window.

The problem is that an NSEvent can sometimes have a nil window
([NSEvent window] can sometimes return nil).  When this happens,
locationInWindow returns a point in screen coordinates, not in the
coordinates of any window.

Current Cocoa widgets code doesn't anticipate this possibility.

Apple's doc on [NSEvent locationInWindow] says (in part):

"With NSMouseMoved and possibly other events, the receiver can have a
nil window (that is, window returns nil). In this case,
locationInWindow returns the event location in screen coordinates."

I'm not currently aware of any practical consequences of this mistake
(I don't currently know of any particular examples where the current
code returns an incorrect value).  But it's not too difficult to
imagine one.

In my next patch I'll post a patch that changes how locationInWindow
is used in Cocoa widgets.

Comment 1

[Steven Michaud [:smichaud] (Retired)]

Attachment: Proposed patch

This patch deals with the possibility that an NSEvent object might
have a nil window, and also with the possibility that the NSEvent's
window (non-nil window) might be different from the one we expect.

Comment 2

[Steven Michaud [:smichaud] (Retired)]

Here's a tryserver build made with my patch from comment #1:

https://build.mozilla.org/tryserver-builds/2008-03-27_16:[e-mail]-bugzilla425526/[e-mail]

Comment 3

[Samuel Sidler (old account; do not CC)]

Looks like this got missed for 1.9 and I don't see why we should take it in a security/stability release, but let's get it back on the radar for 1.9.1...

Comment 4

[Smokey Ardisson (offline for a while; not following bugs - do not email)]

Steven, is this code change still valid/needed?

Comment 5

[Steven Michaud [:smichaud] (Retired)]

My patch is long out of date.  But the underlying problem still exists
-- [NSEvent locationInWindow] is still being used without checking
whether [NSEvent window] returns nil.

Comment 6

[Steven Michaud [:smichaud] (Retired)]

We still call -[NSEvent locationInWindow] in several places, and still never check whether -[NSEvent window] returns NULL.  But it's hard to say whether this is causing trouble.

In one case we do prevent -[NSEvent locationInWindow] from being used for NSMouseMoved events:

https://hg.mozilla.org/mozilla-central/annotate/5d6e0d038f95/widget/cocoa/nsCocoaUtils.mm#l110

But otherwise we don't pay any explicit attention to the problem.

I'm never going to have time for this bug.  But it might be a "good first bug" or "good next bug".  And I'd be happy to review a patch for it.

Comment 7

[Steven Michaud [:smichaud] (Retired)]

For what it's worth, the text I quoted in comment #0 is still present in Apple's current documentation for -[NSEvent locationInWindow]:

https://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSEvent_Class/index.html#//apple_ref/occ/instm/NSEvent/locationInWindow