1541839 - HTTP header value not fully readable

Status: NEW

(DevTools :: Netmonitor, enhancement, P3)

Description

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

Attachment: image.png

STR:

1. Open DevTools Toolbox and select the Network panel
2. Load https://cspvalidator.org/
3. Select the first document request in order to see the Headers side bar
4. Check value of the content-security-policy header, it's not fully visible (it's shortened using ellipsis in the middle, see the screenshot) and there is no way to see the full value.

Expected:
The user should be able to see the full value e.g. in a tooltip. Or any better idea?

Honza

Comment 1

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

Related comment:
https://bugzilla.mozilla.org/show_bug.cgi?id=1447267#c8

Honza

Comment 2

[Dhyey Thakore [:dhyey35]]

@Honza can I get assigned to this bug ?

Comment 3

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

Yes.

What we want here is a tooltip - implemented as part of the PropertiesView, so we support all:

• Headers panel
• Cookies
• Params

Plus a test

Thanks,
Honza

Comment 4

[Dhyey Thakore [:dhyey35]]

Attachment: Bug 1541839 - HTTP header value not fully readable

Comment 5

[Dhyey Thakore [:dhyey35]]

@Honza As StringRep - which renders the span containing value, already has a title option I just added a title in HeadersPanel.

Can you please let me know how can I implement this in PropertiesView as it just calls the renderValue method - https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/PropertiesView.js#258 ?

Comment 6

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

Thanks for working on this!

(In reply to Dhyey Thakore [:dhyey35] from comment #5)

@Honza As StringRep - which renders the span containing value, already has a title option I just added a title in HeadersPanel.

I see, the Headers panel customizes the rendering, so your change is correct.

Can you please let me know how can I implement this in PropertiesView as it just calls the renderValue method - https://searchfox.org/mozilla-central/source/devtools/client/netmonitor/src/components/PropertiesView.js#258 ?

The PropertiesView should do similar thing like the HeadersPanel - it'll be consumed by Cookies and Params panel sine they don't do custom rendering. You should get the value from the member object

In Properties View:

  return Rep(Object.assign(props, {
    member: Object.assign({}, member, { open: false }),
    mode: MODE.TINY,
    cropLimit: this.props.cropLimit,
    noGrip: true,
    title: member.value
  }));

This also needs a test.

Honza

Comment 7

[Dhyey Thakore [:dhyey35]]

PropertiesView is used in:

1. Headers
2. Params
3. Cookies
4. Cache
5. Security
6. Response ( no tooltip needed )

In first three we want a tooltip, Cache will also get a tooltip as it relies on renderValueWithRep of PropertiesView, I think we want that.

Q1: Do we want it for Security ?

It has it's own renderValue with a input box whose value is selected on click ( for ease in copy )

Screenshot: https://imgur.com/a/HmQX846

Q2: Do I need to create a new file for test or do you have any file which I can extend ?

I have browser_net_simple-request-details.js in mind but it doesn't have Cache tab ( nor Security if we decide to add tooltip for it ).

Comment 8

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

Attachment: image.png

Thanks for the update!

(In reply to Dhyey Thakore [:dhyey35] from comment #7)

PropertiesView is used in:

1. Headers
2. Params
3. Cookies
4. Cache
5. Security
6. Response ( no tooltip needed )

In first three we want a tooltip, Cache will also get a tooltip as it relies on renderValueWithRep of PropertiesView, I think we want that.

Q1: Do we want it for Security ?

It has it's own renderValue with a input box whose value is selected on click ( for ease in copy )

Screenshot: https://imgur.com/a/HmQX846

I think that it could be useful for Security too. See my screenshot, if the width of the side panel is small the value doesn't have to be fully visible.

Q2: Do I need to create a new file for test or do you have any file which I can extend ?

I have browser_net_simple-request-details.js in mind but it doesn't have Cache tab ( nor Security if we decide to add tooltip for it ).

Good point, what about creating a new test dedicated for testing tooltips in all side panel that support it?
Consequently we can remove the testing from the existing browser_net_simple-request-details.js file (which is getting a little too big anyway).

Honza

Comment 9

[Dhyey Thakore [:dhyey35]]

Ok I will create a new test file. Should I create a new SJS file for https server as I will need to test the Security tab too ?

Comment 10

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

(In reply to Dhyey Thakore [:dhyey35] from comment #9)

Ok I will create a new test file. Should I create a new SJS file for https server as I will need to test the Security tab too ?

Yep, sounds good to me

Honza

Comment 11

[Patrick Brosset <:pbro>]

Hi Dhyey, just checking in on this bug. There hasn't been activity on it over the past 2 months. It looks like Honza was asking for a rebase of the patch in phabricator.
Are you still interested in working on this bug? If so and you just need more time, that's great. If not, let us know so we can make this bug available for others to fix.

Comment 12

[Dhyey Thakore [:dhyey35]]

Hi Patrick, right now I am working on the Inline Variable Preview in the Debugger tab, will fix this one after that.

Comment 13

[Patrick Brosset <:pbro>]

Unassigning for now since there hasn't been any activity here or on phabricator over the past 4 months.
Please feel free to claim it again once you resume work on it.

Comment 14

[Matti Eteläperä]

Attachment: jwt-bug-report.PNG

Comment 15

[Matti Eteläperä]

Comment on attachment 9243637 [details]
jwt-bug-report.PNG

I ran into this problem first hand. We have an Authorize request header with a long Bearer JWT. I wanted to copy the token to the clipboard without the "Bearer " prefix, so I simply selected the JWT part, pasted it to a validator and couldn't understand why it was invalid.

The reason is that Headers seem to be truncated to 1024 bytes. In my case it is near impossible to spot the "..." character in the middle of the JWT.

1. The tooltip here would help, but a clearer notification that the header is truncated is needed.
2. Placing the "..." at the end of the truncated header value would be much more informative. Why is it in the middle of the header in the first place?

Comment 16

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

1. The tooltip here would help, but a clearer notification that the header is truncated is needed.

I agree, the "..." character isn't much visible.

2. Placing the "..." at the end of the truncated header value would be much more informative. Why is it in the middle of the header in the first place?

I don't recall the reasoning, but it might have been that sometimes the end of the string can be actually a bit more important than the value in middle.

In any case, it looks like that users are copying the values quite often and this truncating logic represents a blocker.

What if we removed the truncating entirely? (Chrome also doesn't truncate)? Or increased the max limit significantly?

Or perhaps we could make it clearer that the truncation happened and if it does we would allow the user to expand the string to see the full actual value - e.g. by clicking at the "..." character or any other visual element we'd added.

@Bomsy WDYT?

Honza

Comment 17

[Hubert Boma Manilla (:bomsy)]

What if we removed the truncating entirely? (Chrome also doesn't truncate)? Or increased the max limit significantly?

There is current a cropLimit of 1024 , we could increase it significantly or just remove it as an initial fix.
https://searchfox.org/mozilla-central/rev/75e9d727ce5ba2c14653cf8fb0f1367f085271b7/devtools/client/netmonitor/src/components/request-details/PropertiesView.js#76

Or perhaps we could make it clearer that the truncation happened and if it does we would allow the user to expand the string to see the full actual value - e.g. by clicking at the "..." character or any other visual element we'd added.

I like this idea. I'll The String Rep handles the formatting we can extend it functionality to support this.

Thanks