VariablesView `Editable#_onCleanup` is not called when it should be

RESOLVED FIXED in Firefox 29

Status

defect
P1
normal
RESOLVED FIXED
6 years ago
11 months ago

People

(Reporter: vporof, Assigned: bbenvie)

Tracking

unspecified
Firefox 29

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

6 years ago
Regression after bug 808371.

STR:
1. Open debugger on http://htmlpad.org/debugger/
2. Add a watch expression (ex: "window")
3. Double click the expression in the variables view
4. Press RETURN

The value hides.
https://bugzilla.mozilla.org/show_bug.cgi?id=808371#c46
(Reporter)

Updated

6 years ago
Flags: needinfo?(bbenvie)
(Reporter)

Updated

5 years ago
Summary: Bug 947611 - Double clicking on watch expressions in the debugger and then unfocusing the text intput hides their value → Double clicking on watch expressions in the debugger and then unfocusing the text intput hides their value
(Reporter)

Updated

5 years ago
Blocks: 808371
(Assignee)

Comment 1

5 years ago
Regression from bug 808371 probably. I'll look into it.
Assignee: nobody → bbenvie
Status: NEW → ASSIGNED
Flags: needinfo?(bbenvie)
Priority: -- → P1
(Assignee)

Comment 2

5 years ago
This patch should fix the issue. `_onCleanup` was not being called at the appropriate time.
Attachment #8347512 - Flags: review?(vporof)
(Assignee)

Updated

5 years ago
Duplicate of this bug: 947611
(Assignee)

Updated

5 years ago
Summary: Double clicking on watch expressions in the debugger and then unfocusing the text intput hides their value → VariablesView `Editable#_onCleanup` is not called when it should be
(Reporter)

Comment 4

5 years ago
Comment on attachment 8347512 [details] [diff] [review]
variables-view-onCleanup.patch

Review of attachment 8347512 [details] [diff] [review]:
-----------------------------------------------------------------

Please add a test.
Attachment #8347512 - Flags: review?(vporof) → review-
(Reporter)

Comment 5

5 years ago
Any progress with the test Benvie? This bug makes it really hard to work with watch expressions in the debugger.
Flags: needinfo?(bbenvie)
(Assignee)

Comment 6

5 years ago
Sorry about that! This fell off my radar with all the promise stuff. I'll work on this today.
Flags: needinfo?(bbenvie)
(Assignee)

Comment 7

5 years ago
Adds a test.
Attachment #8347512 - Attachment is obsolete: true
Attachment #8361296 - Flags: review?(vporof)
(Reporter)

Comment 9

5 years ago
Comment on attachment 8361296 [details] [diff] [review]
variables-view-onCleanup.patch

Review of attachment 8361296 [details] [diff] [review]:
-----------------------------------------------------------------

r+ with the comments addressed and a green try.

::: browser/devtools/debugger/test/browser_dbg_variables-view-edit-cancel.js
@@ +8,5 @@
> +
> +const TAB_URL = EXAMPLE_URL + "doc_watch-expressions.html";
> +
> +
> +let {console} = Cu.import("resource://gre/modules/devtools/Console.jsm", {});

Remove this if not needed.

@@ +16,5 @@
> +    let win = panel.panelWin;
> +    let vars = win.DebuggerView.Variables;
> +
> +    win.DebuggerView.WatchExpressions.addExpression("this");
> +    executeSoon(() => debuggee.ermahgerd());

Please document the reason for this executeSoon.
Attachment #8361296 - Flags: review?(vporof) → review+
(Assignee)

Comment 10

5 years ago
Remove leftover debug line and add a comment explaining executeSoon.
Attachment #8361296 - Attachment is obsolete: true
Attachment #8361308 - Flags: review+
(Reporter)

Comment 11

5 years ago
Comment on attachment 8361308 [details] [diff] [review]
variables-view-onCleanup.patch

Try is all orange :(
Attachment #8361308 - Flags: review+ → review-
(Reporter)

Updated

5 years ago
Flags: needinfo?(bbenvie)
(Assignee)

Comment 12

5 years ago
EditableNameAndValue no longer has an _onCleanup hook, this fixes it.
Attachment #8361308 - Attachment is obsolete: true
Flags: needinfo?(bbenvie)
(Assignee)

Comment 13

5 years ago
Comment on attachment 8361827 [details] [diff] [review]
variables-view-onCleanup.patch

Adding back r+ with fix.
Attachment #8361827 - Flags: review+
https://hg.mozilla.org/mozilla-central/rev/eebd90d1bba5
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29

Updated

11 months ago
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.