725908 - check "enable event handler" checkbox of AccessibleEvents viewer when user starts typing the event handler

Status: RESOLVED FIXED

(Other Applications :: DOM Inspector, defect)

Description

[alexander :surkov (:asurkov)]

to locate "enable event handler" checkbox and "event handler output" area go to AccessibleEvents viewer and then switch to WatchedEvents tab.

it'd be nice if you selected some event in the list, focused "event handler output" and started typing then that checkbox is checked automatically.

Comment 1

[alexander :surkov (:asurkov)]

Attachment: patch

Comment 2

[[e-mail]]

Comment on attachment 595933 [details] [diff] [review]
patch

If you clear the contents of the handler, would you want to automatically disable it?

>-    var updateCheckbox = !updateData || aRowIdx == this.selection.currentIndex;
This breaks clicking on the checkboxes for other rows.

>+                   oninput="viewer.onWatchViewEditorChanged();"/>
Is it likely that the event could fire as the user was typing, and therefore incorrect code could be executed?

Comment 3

[alexander :surkov (:asurkov)]

Attachment: patch2

Comment 4

[[e-mail]]

Comment on attachment 596077 [details] [diff] [review]
patch2

I like this approach :-)

>+    this.mHandlerEditor.addEventListener("blur", this, false);
Probably better to use "change" rather than "blur". r=me with that fixed.

>-    var idx = this.selection.currentIndex;
You end up using this.selection.currentIndex in both event types anyway...

>+    if (aEvent.type == "input") {
>+      var isEnabled = this.mHandlerEditor.value != "";
>+      this.updateHandlerState(isEnabled, this.selection.currentIndex);
[I was considering updating the handler state on the change event too, but I can't convince myself that it wouldn't be too confusing.]

>+      return;
Don't need this, type can't be both input and something else. (Or use switch.)

Comment 5

[alexander :surkov (:asurkov)]

Attachment: patch3

addressed Neil's comments

Comment 6

[alexander :surkov (:asurkov)]

landed http://hg.mozilla.org/dom-inspector/rev/9856b89c71a8