[Webkit-unassigned] [Bug 61834] Web Inspector: CRASH if Expanding Event Listener on document

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=61834





--- Comment #8 from Pavel Feldman <pfeldman at chromium.org>  2011-06-01 22:13:22 PST ---
(From update of attachment 95642)
View in context: https://bugs.webkit.org/attachment.cgi?id=95642&action=review

Here is a number of hints on the present testing framework.

> LayoutTests/http/tests/inspector/elements-test.js:111
> +    InspectorTest.expandSelectedElementEventListeners(function() {

We generally use named functions with meaningless names in the callbacks :)

> LayoutTests/http/tests/inspector/elements-test.js:123
> +    InspectorTest.runAfterPendingDispatches(function() {

I'd replace this with the sniffer (InspectorTest.addSniffer) that would fire when event listeners data hits front-end.

> LayoutTests/http/tests/inspector/elements-test.js:135
> +    InspectorTest.runAfterPendingDispatches(function() {

I don't think you need to run pending dispatches here.

> LayoutTests/http/tests/inspector/elements-test.js:150
> +    InspectorTest.runAfterPendingDispatches(callback);

And here.

> LayoutTests/inspector/elements/event-listener-sidebar-expected.txt:9
> +===== Dump Event Listeners =====

Not sure we need these log entries.

> LayoutTests/inspector/elements/event-listener-sidebar-expected.txt:15
> +(anonymous function)documentisAttribute: falselistenerBody: "function (event) { console.log("click - document - capturing"); }"node: HTMLDocumenttype: "click"useCapture: true

This formatting could get a bit more love (line breaks). Otherwise "nodeisAttribute" and "truelistenerBody" look confusing.

> LayoutTests/inspector/elements/event-listener-sidebar.html:11
> +    button.addEventListener("hover", function(event) { console.log("hover - button - bubbling"); }, false);

Naming the functions would provide even better test coverage.

> LayoutTests/inspector/elements/event-listener-sidebar.html:29
> +            InspectorTest.evaluateInPage("setupEventListeners()", callback);

There is no need for the front-end to be initialized by the time you run setupEventListeners, so you could simply do that all in the onload(), followed with the runTests(); call.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.