Closed Bug 725908 Opened 13 years ago Closed 13 years ago

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

Categories

(Other Applications :: DOM Inspector, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: surkov, Assigned: surkov)

References

Details

Attachments

(1 file, 2 obsolete files)

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.
Attached patch patch (obsolete) — Splinter Review
Assignee: nobody → surkov.alexander
Status: NEW → ASSIGNED
Attachment #595933 - Flags: review?(neil)
Summary: check "enable event target" checkbox of AccessibleEvents viewer when user starts typing the event handler → check "enable event handler" checkbox of AccessibleEvents viewer when user starts typing the event handler
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?
Attachment #595933 - Flags: review?(neil) → review-
Attached patch patch2 (obsolete) — Splinter Review
Attachment #595933 - Attachment is obsolete: true
Attachment #596077 - Flags: review?(neil)
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.)
Attachment #596077 - Flags: review?(neil) → review+
Attached patch patch3Splinter Review
addressed Neil's comments
Attachment #596077 - Attachment is obsolete: true
landed http://hg.mozilla.org/dom-inspector/rev/9856b89c71a8
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Blocks: 727380
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: