114366 - UMR in nsHTMLInputElement.cpp testing event flags

Status: VERIFIED FIXED

(Core :: DOM: UI Events & Focus Handling, defect)

Description

[John Morrison]

running purify, I get a lot of UMR warnings from this bit of code, 
called from a variety of different locations (full purify logs 
to be attached). This happens when typing in the urlbar, or focus/blurring
the window and the input element.

aEvent->flags is 0xdeadbeef running under VC6, and I don't really see 
where that member is initialized prior to its use, but maybe I'm just 
missing something here.

1151 
1152   // If NS_EVENT_FLAG_NO_CONTENT_DISPATCH is set we will not allow content 
to handle
1153   // this event.  But to allow middle mouse button paste to work we must 
allow 
1154   // middle clicks to go to text fields anyway.
1155   PRBool noContentDispatch = aEvent->flags & 
NS_EVENT_FLAG_NO_CONTENT_DISPATCH;
1156   if ((type == NS_FORM_INPUT_TEXT || type == NS_FORM_INPUT_PASSWORD) &&
1157       aEvent->message == NS_MOUSE_MIDDLE_CLICK) {
1158     aEvent->flags &= ~NS_EVENT_FLAG_NO_CONTENT_DISPATCH;
1159   }
1160

Comment 1

[John Morrison]

Attachment: purify logs for a variety of call stacks that trigger this warning

Comment 2

[[e-mail] (gone - use [e-mail] instead)]

jst might be able/willing to fix this bug - he was gracious enough to fix bug 
109487 for me.

Comment 3

[Brendan Eich [:brendan]]

Good grief.  Can't we fix this for 0.9.9?  Who knows what strange bugs it might
be causing?

/be

Comment 4

[Johnny Stenback (:jst)]

Fixing this will only fix one of many identical problems, see my comment in
http://bugzilla.mozilla.org/show_bug.cgi?id=109487#c1 

Comment 5

[Radha on family leave (not reading bugmail)]

Taking from joki.

Comment 6

[Radha on family leave (not reading bugmail)]

Attachment: Initial patch

This patch takes care of most of the situations listed in the purify logs. I'm
not certain that stack traces 1,2, and 9 are covered by this patch. My purify
builds consistently crash, preventing me from verifying that. jrgm, if you can
provide me a longer stack trace, I can cover those too.  I identified a few
more places where this could be fixed. But I don't want to introduce
unnecessary changes to the tree at this time.  Mozilla seems to work fine with
this patch.

Comment 7

[Johnny Stenback (:jst)]

Joki is actually fixing all these by adding a constructor to nsEvent, that'll
take care of every instance of code that has this same problem. Joki, do we
still want this change? I'd say we take your cover-all change in stead and leave
this one out.

Comment 8

[joki (gone)]

Attachment: patch with constructor for nsEvent

Comment 9

[joki (gone)]

Yeah, the patch is attached.  As jst said, this'll cover the individual cases so
we can just go with the constructor patch if we want.

Comment 10

[Radha on family leave (not reading bugmail)]

I think joki should own this, since he has a better patch to take care of this. 

Comment 11

[Johnny Stenback (:jst)]

Comment on attachment 76261 [details] [diff] [review]
patch with constructor for nsEvent

sr=jst

Comment 12

[saari (gone)]

Comment on attachment 76261 [details] [diff] [review]
patch with constructor for nsEvent

r=saari, straight forward enough

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Comment on attachment 76261 [details] [diff] [review]
patch with constructor for nsEvent

a=roc+moz

Comment 14

[joki (gone)]

Fix checked in.

Comment 15

[John Morrison]

verify fixed in a purify run (for pure pedantry).