1358192 - Filtered arrays in JSON viewer cannot be expanded

Status: ASSIGNED

(DevTools :: JSON Viewer, defect, P3)

Description

[Dan Callahan [:callahad]]

Steps to reproduce:

1. Visit https://accounts.google.com/.well-known/openid-configuration
2. Enter "supported" in the Filter JSON field to isolate several properties

Expected Result:

- The properties are enumerated, and can be expanded to display their values.

What Actually Happens:

- The properties are enumerated, but there is no way to display their values.

This is in contrast to string values, which are still displayed as long as their associated property is displayed per the filter (try filtering on "endpoint" at the above URL)

( Reported by Elijah at https://hacks.mozilla.org/2017/04/firefox-53-quantum-compositor-compact-themes-css-masks-and-more/#comment-21092 )

Comment 1

[jan-jan]

Attachment: Bug 1358192 - Filtered arrays in JSON viewer cannot be expanded

Solves problem, by being aware of path, so parent key can filter all children

Review commit: https://reviewboard.mozilla.org/r/245308/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/245308/

Comment 2

[Oriol Brufau [:Oriol]]

Concatenating path, name and value doesn't seem right to me. I would do something like:

  let search = this.props.searchFilter.toLowerCase();
  return [path, object.name, object.value].some(function(text) {
    return text.toLowerCase().includes(search);
  });

But I'm not much sure that allowing any path that contains the filter is the good thing to do. Lots of non-matching descendants may be included, and this can make the filter useless.
I think the code should find the innermost nodes that match, and collapse them. Then allow the user to expand to see descendants, but don't show them by default. Removing the filter should probably expand again the items that were collapsed by the filter. Not much sure if this temporary collapse should only affect the innermost nodes that match, or also their descendants.

Comment 3

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Comment on attachment 8979045 [details]
Bug 1358192 - Filtered arrays in JSON viewer cannot be expanded

https://reviewboard.mozilla.org/r/245308/#review252128

Thanks for working on this!

Please see my inline comment (following the suggestion from Oriol)


> I think the code should find the innermost nodes that match, and collapse them. Then allow the user to expand to see descendants, but don't show them by default. Removing the filter should probably expand again the items that were collapsed by the filter. Not much sure if this temporary collapse should only affect the innermost nodes that match, or also their descendants.

The rest of the suggestion can be done in a follow up.
@Oriol: can you please file a new bug for it and add
detailed functional description? (we might want to
use the new bug for a disscussion)

Thanks!
Honza

::: devtools/client/jsonview/components/JsonPanel.js:76
(Diff revision 1)
> -    onFilter(object) {
> +    onFilter(object, path) {
>        if (!this.props.searchFilter) {
>          return true;
>        }
>  
> -      let json = object.name + JSON.stringify(object.value);
> +      let json = path + object.name + JSON.stringify(object.value);

I agree with Oriol, this should be replaced by something like as follow:

```js
let search = this.props.searchFilter.toLowerCase();
return [path, object.name, object.value].some(function(text) {
  return text.toLowerCase().includes(search);
});
```

Comment 4

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Try push with the current patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66673c66e7a0bdafff37f2ca79b3413df7336975

Honza

Comment 5

[Jan Honza Odvarko [:Honza] (always need-info? me)]

I am seeing one failing test 
TEST-UNEXPECTED-FAIL | devtools/client/jsonview/test/browser_jsonview_filter.js | Uncaught exception - Error: The JSON Viewer did not load.

Try results:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66673c66e7a0bdafff37f2ca79b3413df7336975&selectedJob=179802586

Honza

Comment 6

[BugBot [:suhaib / :marco/ :calixte]]

Clear a needinfo that is pending on an inactive user.

Inactive users most likely will not respond; if the missing information is essential and cannot be collected another way, the bug maybe should be closed as INCOMPLETE.

For more information, please visit auto_nag documentation.

Comment 7

[Zac Svoboda :zacnomore]

This looks to be a variation on bug 1217131, both have some work done in them.
@Honza, can we close one or the other?

Comment 9

[Jan Honza Odvarko [:Honza] (always need-info? me)]

Yes, I closed the bug 1217131

Anyone working on fixing this bug can also look at patch introduced here, it might still be a good inspiration:
https://bugzilla.mozilla.org/show_bug.cgi?id=1217131#c4

Comment 10

[Zac Svoboda :zacnomore]

Attachment: Bug 1358192 - Highlight filter matches, keep children in JSON viewer r?Honza

Comment 11

[BugBot [:suhaib / :marco/ :calixte]]

The bug assignee is inactive on Bugzilla, so the assignee is being reset.

Comment 12

[Karan Yadav]

Hello, it seems no one is currently working on this issue. Could you please assign the bug to me?

Comment 13

[Nicolas Chevobbe [:nchevobbe]]

Zac, would you be fine with Karan taking over?

Comment 14

[Zac Svoboda :zacnomore]

Go for it!

Comment 15

[Nicolas Chevobbe [:nchevobbe]]

Thanks Zac!
Karan, it's yours :)

Comment 17

[Karan Yadav]

Attachment: Bug 1358192 - [devtools] highlight filtered row in json viewer and add navigation buttons. r=#devtools-reviewers