Closed Bug 318678 Opened 15 years ago Closed 15 years ago

[FIX]events registered via divElem.onmouseover and divElem.onclick do not trigger

Categories

(Core :: DOM: Core & HTML, defect, P1)

defect

Tracking

()

RESOLVED FIXED
mozilla1.9alpha1

People

(Reporter: vi, Assigned: bzbarsky)

References

()

Details

(Keywords: fixed1.8.0.1, fixed1.8.1, regression)

Attachments

(2 files)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8) Gecko/20051201 Firefox/1.5
Build Identifier: Mozilla/5.0 (X11; U; Linux i686 (x86_64); en-US; rv:1.8) Gecko/20051201 Firefox/1.5

Go to http://map.sh.nu/prototype4/ and allow popups for the site. When the site loads, a "debug" window which is created entirely in DOM will pop up. In this window there is the text "scalebarImg" and "tiles" at the top left. These are divs with text nodes appended. The divs are created from the source here:

http://map.sh.nu/w/prototype4/debug.txt

The 4 events are registered on lines 122, 125, 141, and 144. In firefox 1.0.7 this works perfectly. In firefox 1.5 it does not. If I use venkman to "slow down" the execution of the script by adding breakpoints the above lines, the onclick events begin to work, but the onmouseover still does not.

The javascript console doesn't report anything when I mouse over or click on the divs elements.

Even though I'm on Linux X86_64, I've confirmed this on Linux x86 and windows x86.

Reproducible: Always
Taking bug, but bz, feel free to have a look if you've got some cycles.
Assignee: nobody → jst
OS: Linux → All
Hardware: PC → All
Version: unspecified → 1.5 Branch
this regressed between 2005-07-30-05 and 2005-07-31-10 (landing of 296639)
Status: UNCONFIRMED → NEW
Component: General → DOM
Depends on: splitwindows
Ever confirmed: true
Keywords: regression
Product: Firefox → Core
QA Contact: general → ian
Version: 1.5 Branch → Trunk
(regrression was for onclick specifically)
So I added some logging to nsEventListenerManager::RegisterScriptEventListener to see what's going on as follows:

  fprintf(stderr, "Registering listener for %s on %p\n", NS_ConvertUTF16toUTF8(type).get(), jsobj);

Here's what I see for the div in question (by matching up pointer values):

Created wrapped native [object HTMLDivElement @ 0xb2b67d18 (native @ 0xb2b67cf0)], flat JSObject is 0xb2b2ca88
Registering listener for onclick on 0xb2b2ca88
Registering listener for onclick on 0xb2b2ca88

and then much later (probably as I click around):

Created wrapped native [object HTMLDivElement @ 0xb2b9a340 (native @ 0xb2b67cf0)], flat JSObject is 0xb2b6f450
Created wrapped native [object HTMLDivElement @ 0xb2b9baf0 (native @ 0xb2b67cf0)], flat JSObject is 0xb3d5b6c0
Created wrapped native [object HTMLDivElement @ 0xb3c16f20 (native @ 0xb2b67cf0)], flat JSObject is 0xb4dfa950

So for some reason we "lose" the original XPCWrappedNative that the event listener is set on... hence no listener firing.

This is all for the onclick listener; the onmouseover listener is a bug in the site (but Daniel, please don't change the code until we can get to the bottom of this, ok?).
Attached patch FixSplinter Review
Fix -- have to use the correct old scope when reparenting wrappers...
Attachment #204852 - Flags: superreview?(jst)
Attachment #204852 - Flags: review?(jst)
I think we want this on the 1.8 branch... I sorta wish I'd followed up more on my questions about this code in bug 296639.  :(
Assignee: jst → bzbarsky
Flags: blocking1.8.1?
Flags: blocking1.8.0.1?
Priority: -- → P1
Summary: events registered via divElem.onmouseover and divElem.onclick do not trigger → [FIX]events registered via divElem.onmouseover and divElem.onclick do not trigger
Target Milestone: --- → mozilla1.9alpha
Comment on attachment 204852 [details] [diff] [review]
Fix

r+sr=jst
Attachment #204852 - Flags: superreview?(jst)
Attachment #204852 - Flags: superreview+
Attachment #204852 - Flags: review?(jst)
Attachment #204852 - Flags: review+
Checked in to trunk.
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment on attachment 204852 [details] [diff] [review]
Fix

I think we need this on the 1.8 branch as soon as we can manage... :(
Attachment #204852 - Flags: approval1.8.0.1?
Comment on attachment 204852 [details] [diff] [review]
Fix

Please land for 1.8.0 and 1.8 branches.  Thanks!
Attachment #204852 - Flags: approval1.8.1+
Attachment #204852 - Flags: approval1.8.0.1?
Attachment #204852 - Flags: approval1.8.0.1+
Could someone please land this for me?  If not, I'll do it in early January...
Flags: blocking1.8.1?
Flags: blocking1.8.1+
Flags: blocking1.8.0.1?
Flags: blocking1.8.0.1+
*** Committing to MOZILLA_1_8_BRANCH... 
Checking in content/base/public/nsContentUtils.h;
new revision: 1.60.4.5; previous revision: 1.60.4.4
Checking in content/base/src/nsContentUtils.cpp;
new revision: 1.107.4.9; previous revision: 1.107.4.8

*** Committing content/base/public/nsContentUtils.h on MOZILLA_1_8_0_BRANCH... 
new revision: 1.60.4.4.2.1; previous revision: 1.60.4.4
*** Committing content/base/src/nsContentUtils.cpp on MOZILLA_1_8_0_BRANCH... 
new revision: 1.107.4.8.2.1; previous revision: 1.107.4.8
Blocks: splitwindows
No longer depends on: splitwindows
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.