Closed
Bug 947612
Opened 11 years ago
Closed 11 years ago
VariablesView `Editable#_onCleanup` is not called when it should be
Categories
(DevTools :: Debugger, defect, P1)
DevTools
Debugger
Tracking
(Not tracked)
RESOLVED
FIXED
Firefox 29
People
(Reporter: vporof, Assigned: bbenvie)
References
Details
Attachments
(1 file, 3 obsolete files)
5.05 KB,
patch
|
bbenvie
:
review+
|
Details | Diff | Splinter Review |
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•11 years ago
|
Flags: needinfo?(bbenvie)
Reporter | ||
Updated•11 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
Assignee | ||
Comment 1•11 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•11 years ago
|
||
This patch should fix the issue. `_onCleanup` was not being called at the appropriate time.
Attachment #8347512 -
Flags: review?(vporof)
Assignee | ||
Updated•11 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•11 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•11 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•11 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•11 years ago
|
||
Adds a test.
Attachment #8347512 -
Attachment is obsolete: true
Attachment #8361296 -
Flags: review?(vporof)
Assignee | ||
Comment 8•11 years ago
|
||
Reporter | ||
Comment 9•11 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•11 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•11 years ago
|
||
Comment on attachment 8361308 [details] [diff] [review]
variables-view-onCleanup.patch
Try is all orange :(
Attachment #8361308 -
Flags: review+ → review-
Reporter | ||
Updated•11 years ago
|
Flags: needinfo?(bbenvie)
Assignee | ||
Comment 12•11 years ago
|
||
EditableNameAndValue no longer has an _onCleanup hook, this fixes it.
Attachment #8361308 -
Attachment is obsolete: true
Flags: needinfo?(bbenvie)
Assignee | ||
Comment 13•11 years ago
|
||
Comment on attachment 8361827 [details] [diff] [review]
variables-view-onCleanup.patch
Adding back r+ with fix.
Attachment #8361827 -
Flags: review+
Assignee | ||
Comment 14•11 years ago
|
||
Whiteboard: [fixed-in-fx-team]
Comment 15•11 years ago
|
||
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Whiteboard: [fixed-in-fx-team]
Target Milestone: --- → Firefox 29
Updated•6 years ago
|
Product: Firefox → DevTools
You need to log in
before you can comment on or make changes to this bug.
Description
•