Closed Bug 1171723 Opened 5 years ago Closed 5 years ago

Clicking the X button after searching in the variables view doesn't clear the filter

Categories

(DevTools :: Object Inspector, defect)

40 Branch
defect
Not set

Tracking

(firefox41 fixed, firefox42 fixed)

RESOLVED FIXED
Firefox 42
Tracking Status
firefox41 --- fixed
firefox42 --- fixed

People

(Reporter: bgrins, Assigned: ochameau)

References

Details

Attachments

(1 file, 2 obsolete files)

STR:

* Run this in the console: let a = {a:1,b:1}; inspect(a)
* Search for 'b' in the variables view search box
* Press the X in the search box

Expected:
All object properties are visible

Actual:
The filter is still applied ('b' is the only prop visible)
See Also: → 1169096
Assignee: nobody → poirot.alex
The xul textbox being used here, doesn't dispatch `input` events when we click on the clear button.
But it does dispatch a `command` event. Other tools are listening to this event.
It also has the benefit to merge multiple keypress into a single event once user stops typing.
(In reply to Alexandre Poirot [:ochameau] from comment #1)
> Created attachment 8617394 [details] [diff] [review]
> Fix clear button in variable view.
> 
> The xul textbox being used here, doesn't dispatch `input` events when we
> click on the clear button.
> But it does dispatch a `command` event. Other tools are listening to this
> event.
> It also has the benefit to merge multiple keypress into a single event once
> user stops typing.

Do they have to press 'enter' for the event to fire, or does it automatically fire after they stop typing for some amount of time?
Status: NEW → ASSIGNED
It automatically fires after you stop typing.
Attachment #8628803 - Flags: review?(bgrinstead)
Comment on attachment 8628803 [details] [diff] [review]
Fix clear button in variable view

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

Looks fine to me.  Needs minor rebasing on the test files

::: browser/devtools/debugger/test/head.js
@@ +495,5 @@
>    aElement.focus();
>    EventUtils.sendString(aText, aElement.ownerDocument.defaultView);
>  }
>  
> +function sendCommand(aElement) {

Could you just call aElement.doCommand() instead of using this function?

@@ +498,5 @@
>  
> +function sendCommand(aElement) {
> +  let doc = aElement.ownerDocument;
> +  let ev = doc.createEvent("XULCommandEvent");
> +  ev.initCommandEvent("command", true, true, doc.defaultView, 0, false, false, false, false, null);

Nit: 80 chars
Attachment #8628803 - Flags: review?(bgrinstead) → review+
https://treeherder.mozilla.org/#/jobs?repo=try&revision=e5f66d1bf589

(In reply to Brian Grinstead [:bgrins] from comment #6)
> Comment on attachment 8628803 [details] [diff] [review]
> ::: browser/devtools/debugger/test/head.js
> @@ +495,5 @@
> >    aElement.focus();
> >    EventUtils.sendString(aText, aElement.ownerDocument.defaultView);
> >  }
> >  
> > +function sendCommand(aElement) {
> 
> Could you just call aElement.doCommand() instead of using this function?
> 

Sure!
sorry had to back this out in https://treeherder.mozilla.org/#/jobs?repo=fx-team&revision=682a89113d92 for timeouts https://treeherder.mozilla.org/logviewer.html#?job_id=3716320&repo=fx-team that started with this changes
Flags: needinfo?(poirot.alex)
https://hg.mozilla.org/mozilla-central/rev/3d91af004e43
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 42
Attachment #8628803 - Attachment is obsolete: true
Flags: needinfo?(poirot.alex)
Comment on attachment 8631541 [details] [diff] [review]
Fix clear button in variable view

Same request as bug 1169096. This patch goes along with it. If we don't include this additional fix, bug 1169096 behaves badly.
Attachment #8631541 - Flags: review+
Attachment #8631541 - Flags: approval-mozilla-aurora?
Comment on attachment 8631541 [details] [diff] [review]
Fix clear button in variable view

Approved as it has automated tests, and been in m-c for a week.
Attachment #8631541 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.