Open
Bug 1358192
Opened 8 years ago
Updated 2 years ago
Filtered arrays in JSON viewer cannot be expanded
Categories
(DevTools :: JSON Viewer, defect, P3)
DevTools
JSON Viewer
Tracking
(Not tracked)
ASSIGNED
People
(Reporter: callahad, Assigned: zacnomore)
References
(Blocks 1 open bug)
Details
(Keywords: DevAdvocacy)
Attachments
(2 files)
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 hidden (mozreview-request) |
Attachment #8979045 -
Flags: review?(odvarko)
Comment 2•6 years ago
|
||
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•6 years ago
|
||
mozreview-review |
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);
});
```
Attachment #8979045 -
Flags: review?(odvarko)
Comment 4•6 years ago
|
||
Try push with the current patch:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=66673c66e7a0bdafff37f2ca79b3413df7336975
Honza
Comment 5•6 years ago
|
||
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
Flags: needinfo?(jan.jan.code)
Updated•6 years ago
|
Product: Firefox → DevTools
Comment 6•2 years ago
|
||
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.
Flags: needinfo?(jan.jan.code)
Updated•2 years ago
|
Priority: -- → P3
Updated•2 years ago
|
Severity: normal → S3
Assignee | ||
Comment 7•2 years ago
|
||
This looks to be a variation on bug 1217131, both have some work done in them.
@Honza, can we close one or the other?
Flags: needinfo?(odvarko)
Comment 9•2 years ago
|
||
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
Flags: needinfo?(odvarko)
Assignee | ||
Updated•2 years ago
|
Assignee: nobody → cczac1
Status: NEW → ASSIGNED
Assignee | ||
Comment 10•2 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•