Open
Bug 1358192
Opened 7 years ago
Updated 23 days ago
Filtered arrays in JSON viewer cannot be expanded
Categories
(DevTools :: JSON Viewer, defect, P3)
DevTools
JSON Viewer
Tracking
(Not tracked)
NEW
People
(Reporter: callahad, Unassigned)
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
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)
Updated•1 year ago
|
Assignee: nobody → cczac1
Status: NEW → ASSIGNED
Comment 10•1 year ago
|
||
Comment 11•23 days ago
|
||
The bug assignee is inactive on Bugzilla, so the assignee is being reset.
Assignee: cczac1 → nobody
Status: ASSIGNED → NEW
You need to log in
before you can comment on or make changes to this bug.
Description
•