Closed Bug 1240344 Opened 5 years ago Closed 5 years ago

[DevTools][Memory] Filter word is gone after change to "Dominators" in select-view

Categories

(DevTools :: Memory, defect, P3)

defect

Tracking

(firefox46 fixed)

RESOLVED FIXED
Firefox 46
Tracking Status
firefox46 --- fixed

People

(Reporter: magicp.jp, Assigned: jdescottes)

References

(Blocks 1 open bug)

Details

Attachments

(2 files, 2 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
Build ID: 20160116030240

Steps to reproduce:

Please refer to attached video.
1. Start latest Nightly
2. Open Developer Tools and enable Memory tool
3. Select Memory tab
4. Run "Take snapshot"
5. Input filter word (e.g. other)
6. Change view from "Aggregate" to "Dominators"
7. Change view from "Dominators" to "Aggregate"


Actual results:

Filter word is gone after change to "Dominators" in select-view. But filter is still enable.


Expected results:

Filter word is not gone after change to "Dominators" in select-view.
Has STR: --- → yes
Component: Untriaged → Developer Tools: Memory
OS: Unspecified → All
Hardware: Unspecified → All
Priority: -- → P3
Assignee: nobody → jdescottes
Attached patch bug1240344.v1.patch (obsolete) — Splinter Review
Bug 1240344 - fix filter propname used when creating toolbar;r=fitzgen

This commit fixes the prop name used when creating the Toolbar component
from app.js : Toolbar expects fitlerString, app was providing filter.

Extended browser_memory_filter_01.js to check the filter value is preserved
after switching views.
Attachment #8710960 - Flags: review?(nfitzgerald)
Comment on attachment 8710960 [details] [diff] [review]
bug1240344.v1.patch

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

Thanks, Julian!

::: devtools/client/memory/app.js
@@ +108,5 @@
>            allocations,
>            inverted,
>            onToggleInverted: () =>
>              dispatch(toggleInvertedAndRefresh(heapWorker)),
> +          filterString: filter,

D'oh!

::: devtools/client/memory/test/browser/browser_memory_filter_01.js
@@ +24,5 @@
> +            });
> +  };
> +  info(`Waiting for dominator trees to be of state: ${expected}`);
> +  return waitUntilState(store, predicate);
> +}

May be worth throwing this into head.js to deter copying or reimplementing this function next time we need this. If you do, please add a short comment describing what it does and @params.

@@ +59,5 @@
> +    "and filter input contains the user value");
> +
> +  ok(true, "check that the filter is preserved after switching view");
> +
> +  dispatch(changeViewAndRefresh(viewState.DOMINATOR_TREE, heapWorker));

Nit: add a comment above this line something like:

// Now switch to a different view, switch back, and assert that the filter
// word is still in place.
Attachment #8710960 - Flags: review?(nfitzgerald) → review+
Attached patch bug1240344.v2.patch (obsolete) — Splinter Review
Attachment #8710960 - Attachment is obsolete: true
Attachment #8711050 - Flags: review+
Forgot one of the review changes.
Attachment #8711050 - Attachment is obsolete: true
Attachment #8711057 - Flags: review+
Thanks for the review!
> 
> May be worth throwing this into head.js to deter copying or reimplementing
> this function next time we need this. If you do, please add a short comment
> describing what it does and @params.
> 

Done! 
For some reason I thought we had many other occurrences of this in the testsuite and wanted to move it to head.js + use it in other tests in a separate bug. But browser_memory_dominator_trees_01.js is the only other test where we use it. So it's in head.js and it is also used by browser_memory_dominator_trees_01.js

> 
> Nit: add a comment above this line something like:
> 
> // Now switch to a different view, switch back, and assert that the filter
> // word is still in place.

Done!
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/c09a55daebd3
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 46
[bugday-20160323]

Status: RESOLVED,FIXED -> UNVERIFIED

Comments: Test successful

STR: clear.


Component: 
Name			Firefox
Version			46.0b9
Build ID		20160322075646
Update Channel          beta
User Agent		Mozilla/5.0 (Windows NT 6.1; WOW64; rv:46.0) Gecko/20100101 Firefox/46.0
OS			Windows 7 SP1 x86_64

Expected Results: 
Yes

Actual Results: 
As expected
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.