Closed Bug 1245303 Opened 9 years ago Closed 7 years ago

JSON Viewer: linkify URLs

Categories

(DevTools :: JSON Viewer, defect, P2)

defect

Tracking

(Not tracked)

RESOLVED WORKSFORME

People

(Reporter: clarkbw, Unassigned)

References

Details

Attachments

(2 files)

User Story As a web developer inspecting a JSON object in the JSON Viewer I would like to be able to click directly on URL values in the objects such that I can navigate to the URL. I don't think we need to be fancy and look within the text, perhaps just checking if a value starts with http(s)?
Assignee: nobody → gtatum
Attached patch JSON Viewer: linkify URLs (deleted) — Splinter Review
I added the link part to the reps/string.js component. It's fairly naive and only matches for if a string starts with http:// or https://, so there is some potential for it to fail. Also the links open up in a new window.
Attachment #8745091 - Flags: review?(odvarko)
Attached video json-links.mov (deleted) —
This video shows off the styling on the various themes of the links.
Comment on attachment 8745091 [details] [diff] [review] JSON Viewer: linkify URLs Review of attachment 8745091 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay here. I am not sure whether we want to change the general StringRep so, it shows a link everywhere across the UI. I think that other parts of our UI don't have to be happy with such behavior. We might want to introduce a new prop for the StringRep component, something like 'linkify'. that would be switched on by the JSON Viewer. Lin, what do you think? What about the Console panel? Could this feature be useful for the Console panel too? Honza
Attachment #8745091 - Flags: review?(odvarko) → review-
If a URL is a property of an object in the console, for example, we linkify that; I'd be surprised if we don't have a globally accessible resource to linkify a string. We also linkify background image URLs and such in the elements panel.
https://dxr.mozilla.org/mozilla-central/source/devtools/client/webconsole/console-output.js#2238-2277 The console tokenizes strings, and then checks each token to see if it's a URL. This could be pulled out and put into a utility function. Is it worth doing that?
Ok, I've ignored this for awhile, but here is my proposal. I'd like to extract out the functionality of linkifying text to a separate shared file. This way it can be done in the context of both the webconsole and the string rep. Then with the string rep, I'd like to make it configurable to linkify text, but have it off by default. Honza: How does this sound?
Flags: needinfo?(odvarko)
(In reply to Greg Tatum [:gregtatum] [@gregtatum] from comment #6) > Ok, I've ignored this for awhile, but here is my proposal. I'd like to > extract out the functionality of linkifying text to a separate shared file. > This way it can be done in the context of both the webconsole and the string > rep. Then with the string rep, I'd like to make it configurable to linkify > text, but have it off by default. > > Honza: How does this sound? Yes, sounds good to me. @Lin what do you think about my comment #3? I.e. having StringRep with new 'linkify' prop? Honza
Flags: needinfo?(odvarko) → needinfo?(lclark)
(In reply to Jan Honza Odvarko [:Honza] from comment #3) > I am not sure whether we want to change the general StringRep so, it shows a > link everywhere across the UI. I think that other parts of our UI don't have > to be happy with such behavior. In what cases do we not want URLs to be linked? If we have a current use case where URLs shouldn't be linked, then adding it as an option makes sense. But if we don't, I'd say we wait to add the option until we have a use case.
Flags: needinfo?(lclark)
(In reply to Lin Clark [:linclark] from comment #8) > (In reply to Jan Honza Odvarko [:Honza] from comment #3) > > I am not sure whether we want to change the general StringRep so, it shows a > > link everywhere across the UI. I think that other parts of our UI don't have > > to be happy with such behavior. > > In what cases do we not want URLs to be linked? I can't think of any place where it would be an issue. > If we have a current use > case where URLs shouldn't be linked, then adding it as an option makes > sense. But if we don't, I'd say we wait to add the option until we have a > use case. Yes, works for me. Honza
Thanks Lin and Honza, I'll probably work again on this next week.
Given the on-going work with reps, I think this bug should get wrapped up in that. I've not been working on that part of the project and am focused on a few other things. I'm going to take myself off of this bug for now.
Assignee: gtatum → nobody
See also bug 1301910 From that report: This is an enhancement request to improve UX in hypermedia APIs. The behavior to open a link remains to be defined and discussed. We have to keep in mind that the user could want to either just select the link (or parts of), open the link in current tab, or open the link in another tab. An early proposal would be to: - keep default behavior on simple click, - open link on Shift-Click, - and open in another tab on Ctrl-(Shift-)Click.
Seems to work now thanks to Bug 1386525. Test case: data:application/json,{"url":"http://mozilla.com"} In the JSONViewer this renders plain working links (i.e. single click navigate *in* the page, middle-click/cmd+click open in a new tab). Honza, is this enough to close this bug ? Or do you want to tweak the behavior ?
Flags: needinfo?(odvarko)
(In reply to Nicolas Chevobbe [:nchevobbe] from comment #14) > Seems to work now thanks to Bug 1386525. > Test case: data:application/json,{"url":"http://mozilla.com"} > > In the JSONViewer this renders plain working links (i.e. single click > navigate *in* the page, middle-click/cmd+click open in a new tab). > > Honza, is this enough to close this bug ? Or do you want to tweak the > behavior ? Great, I think this can be closed. Honza
Status: NEW → RESOLVED
Closed: 7 years ago
Flags: needinfo?(odvarko)
Resolution: --- → WORKSFORME
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: