Summary: | mouseEvent fires mutiple times in LayoutTests/svg/custom/use-instanceRoot-as-event-target.xhtml | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robert Hogan <robert> | ||||||||||
Component: | WebCore JavaScript | Assignee: | Nobody <webkit-unassigned> | ||||||||||
Status: | RESOLVED DUPLICATE | ||||||||||||
Severity: | Normal | CC: | commit-queue, eric, webkit.review.bot, zimmermann | ||||||||||
Priority: | P2 | ||||||||||||
Version: | 528+ (Nightly build) | ||||||||||||
Hardware: | Other | ||||||||||||
OS: | OS X 10.5 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 32437 | ||||||||||||
Attachments: |
|
Description
Robert Hogan
2009-12-14 08:12:47 PST
Created attachment 44798 [details]
gzipped backtrace
Created attachment 44800 [details]
Update Skipped Lists for Qt, Win and GTK
style-queue ran check-webkit-style on attachment 44800 [details] without any errors.
Comment on attachment 44800 [details]
Update Skipped Lists for Qt, Win and GTK
Rejecting patch 44800 from commit-queue.
Failed to run "['/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', '--reviewer', 'Darin Adler', '--force']" exit_code: 1
Last 500 characters of output:
-----------------------
patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/platform/gtk/Skipped
Hunk #2 succeeded at 5730 (offset -3 lines).
patching file LayoutTests/platform/qt/Skipped
Hunk #1 FAILED at 5187.
1 out of 1 hunk FAILED -- saving rejects to file LayoutTests/platform/qt/Skipped.rej
patching file LayoutTests/platform/win/Skipped
Hunk #2 FAILED at 723.
1 out of 2 hunks FAILED -- saving rejects to file LayoutTests/platform/win/Skipped.rej
Created attachment 44816 [details]
Updated patch
Comment on attachment 44816 [details]
Updated patch
Those paths look wrong.
They should not include "LayoutTests/" or?
Created attachment 44819 [details]
Corrected patch
They were - sorry about that.
Comment on attachment 44819 [details]
Corrected patch
OK.
Comment on attachment 44819 [details] Corrected patch Clearing flags on attachment: 44819 Committed r52117: <http://trac.webkit.org/changeset/52117> All reviewed patches have been landed. Closing bug. Needs to stay open to track fix. Comment on attachment 44800 [details]
Update Skipped Lists for Qt, Win and GTK
Clearing Darin's r+ on this obsolete patch.
After a bit more digging it appears that you can force this test to pass in Qt/Gtk if you remove the following line: useElement.instanceRoot.addEventListener("click", eventHandler, false); It will also pass if you also remove the line above it (leaving both lines commented out): rectElement.removeEventListener("click", eventHandler, false); With the unmodified test, printing the timestamp of each event that prints the final test shows that both rectElement and useElement fire an event on fireSimulatedMouseClick(). If you don't add an event listener to useElement it passes. And if you try to remove the rectElement listener it still passes, when it should really timeout. The multiple outputs of test 10 under Qt/GTK are happening because the 'click' event listener associated with rectElement cannot be removed for some reason. (It's defined in the SVG markup rather than added dynamically, which must be the problem). It could be that event listeners I suspect this is only passing for other ports due to notifyDone() causing the DRTs to finish early rather than wait for all output. Unfortunately, gdb segfaults on --svg builds for me at the moment so I can't investigate further. notifyDone() does cause the testing to end right then. If it doesn't for the Qt DRT, then that's a bug. We might need to better document what notifyDone() does. (In reply to comment #14) > notifyDone() does cause the testing to end right then. If it doesn't for the > Qt DRT, then that's a bug. We might need to better document what notifyDone() > does. In this case I think that the DRT sticking around for all output reveals that the test is actually failing. The detail for the penultimate test is "Test #9: Verify that the original click event listener got removed and a new one attached to the use root SVGElementInstance". If you remove one of the lines that set up test 9: rectElement.removeEventListener("click", eventHandler, false); //useElement.instanceRoot.addEventListener("click", eventHandler, false); it still passes, when it shouldn't. Because the last few tests depend on adding/removing 'click' events, and removing the click event defined in markup has no effect, the 'click' event ends up firing multiple times for each click - when the test comes to an end, the event handler function in the test is still processing the events and sends them all to the last test in the script: function eventHandler(evt) { if (evt.type != nextEventType) return; currentEvent = evt; eval(eventNotification); } Modiying the above as follows also 'fixes' the test by choking off these surplus events that shouldn't be happening. function eventHandler(evt) { if (evt.type != nextEventType) return; nextEventType = undefined; currentEvent = evt; eval(eventNotification); } |