1314184 - Developer Tools - Network Shows Unescaped Encoded URI Component in Request URL

Status: NEW

(DevTools :: Netmonitor, enhancement, P3)

Description

[u564464]

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:49.0) Gecko/20100101 Firefox/49.0
Build ID: 20161019084923

Steps to reproduce:

1. Open the developer Network tab (Inspect Element -> Network)
2. Visit a URL that contains an encoded URI component, e.g. https://www.facebook.com/l.php?u=http%3A%2F%2Fwww.google.com%2F


Actual results:

The request "File" column field and the Headers "Request URL" shows an unescaped form of the request's encoded URI component, "l.php?u=http://www.google.com/"

However, clicking "Edit and Resend" in the request details shows the literal, encoded request URI within the "New Request" URL text field.


Expected results:

I would expect the Network headers "Request URL" and File fields to show the literal request "l.php?u=http%3A%2F%2Fwww.google.com%2F" with the encoded URI component, not an unescaped form of the request's URI parameter.

This caused confusion for me when debugging code that used a request requiring an encoded URI component.

Comment 1

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

(In reply to spammonster2011 from comment #0)
> I would expect the Network headers "Request URL" and File fields to show the
> literal request "l.php?u=http%3A%2F%2Fwww.google.com%2F" with the encoded
> URI component, not an unescaped form of the request's URI parameter.

Yes, I agree.

Thanks for the report,
Honza

Comment 3

[[e-mail]]

Could put Bug 1414981 test cases in concern

When we receive http://www.example.com/%C3%A4/css/syno.css?bob=true#foo"

Should show

http://www.example.com/%C3%A4/css/syno.css?bob=true

Not show

http://www.example.com/ä/css/syno.css?bob=true

Comment 4

[[e-mail]]

Attachment: Bug 1314184 - should show origin request url instead of the decoded url;

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

Comment 5

[[e-mail]]

Comment on attachment 8937660 [details]
Bug 1314184 - should show origin request url instead of the decoded url;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208368/diff/1-2/

Comment 6

[[e-mail]]

Passing origin url in urlDetails, so we can show the origin url on headers panel.


The patch only did the manual test because the test framework seems not support non-ascii folder name.
When I add test files in browser.ini like

```
  ä/html_status-codes-test-page.html
  ä/sjs_status-codes-test-server.sjs
```

Test framework report:

UnicodeDecodeError: 'ascii' codec can't decode byte 0xc3 in position 0: ordinal not in range(128)

Comment 7

[[e-mail]]

Comment on attachment 8937660 [details]
Bug 1314184 - should show origin request url instead of the decoded url;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/208368/diff/2-3/

Comment 8

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

Attachment: encoding.png

Fred, after applying the patch, I am still seeing unescaped form of the request's encoded URI component.

What am I doing wrong?

Honza

Comment 9

[[e-mail]]

Ooh I missed the file column part but just fixed the `request url` in header's sidepanel.
Will add file column part in PR.

Comment 10

[[e-mail]]

After tracing requestColumnFile I found `urlDetails` is mainly used for showing decoded url for the file column.
`Showing decoded url in file column` looks like a feature to me.

And I think this feature is pretty user friendly when people is new to network monitor and the decoded form of URL looks more eye-friendly than the encoded one.


So I tend to only patch the request URL in header's panel. Therefore if a user want to copy the link, they can copy the right encoded URL in header's panel.

u564464, Honza, how do you think?
(Sorry I can't ni u564464@ since the account is disabled)

Comment 11

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

(In reply to Fred Lin [:gasolin] from comment #10)
> After tracing requestColumnFile I found `urlDetails` is mainly used for
> showing decoded url for the file column.
> `Showing decoded url in file column` looks like a feature to me.
> 
> And I think this feature is pretty user friendly when people is new to
> network monitor and the decoded form of URL looks more eye-friendly than the
> encoded one.
> 
> 
> So I tend to only patch the request URL in header's panel. Therefore if a
> user want to copy the link, they can copy the right encoded URL in header's
> panel.
> 
> u564464, Honza, how do you think?
Summary from today's meeting:
1) Keep URLs in the File column as is (decoded)
2) Decode also the URL rendered in Headers panel (for consistency)
3) Introduce a new button 'Raw URL' displayed next
to the URL in Headers panel (to see raw data just like in case of 'Raw Headers')

Honza

Comment 12

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

Comment on attachment 8937660 [details]
Bug 1314184 - should show origin request url instead of the decoded url;

https://reviewboard.mozilla.org/r/208368/#review216614

Removing myself from the review for now (waiting till the suggestions above are ready)

Honza

Comment 13

[[e-mail]]

Attachment: toggle between Request URL / Raw URL

Victoria, we'd like your help if the new UI proposal works for you.


As Comment 11 we'd like to add a feature to show either 

- `Raw URL`(encoded URL, you'll see /%C3%A4%C3%A4/ in URL form), or 
- `Request URL`(decoded URL, you'll see /ää/ in URL form)

The first line in headers panel will show accordingly when user click the `Raw URL`/`Request URL` button besides the Request URL column.

Check the gif for how it could act like. This is just a quick prototype, feel free to propose other UI design.

Another question is if user navigate through requests, should we remember user previously selected `Request URL` or `Raw URL` and always show their preferred form?

Comment 14

[Victoria Wang [:victoria]]

This looks good! Only concern is that it's a bit unfortunate to take up some of the horizontal space there since the URL is often too long to be fully shown already. One solution could be making the current "Request URL" label was a simple dropdown (could look like the throttle menu in RDM), and onclick would show a menu with Request URL and Raw URL. Seems pretty nonstandard though to overload a label with this capacity - I can't think of another UI that does this off the top of my head.

Maybe we could only show the button if we know there are special characters in the URL?

Also, I wonder if the button should say Encoded/Decoded - seems like a more common way to describe these from my Ruby days, but maybe it's more confusing.

And yes, persisting it sounds good - I'm assuming users would usually want the same type of URL as they navigate through requests.

Comment 15

[Victoria Wang [:victoria]]

Comment on attachment 8941317 [details]
toggle between Request URL / Raw URL

wrote review as comment

Comment 17

[Patrick Brosset <:pbro>]

This bug has not been updated in the last 6 months. Resetting the assignee field.
Please, feel free to pick it up again and add a comment outlining your plans for it if you do still intend to work on it.
This is just trying to clean our backlog of bugs and make bugs available for people.

Comment 18

[Hengyeow (:tanhengyeow)]

Hi Honza

This is related to Bug 1491008.

Comment 19

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

@tanhengyeow, thanks for the pointer!

@digitarald: Please see this attachment:
https://bugzilla.mozilla.org/attachment.cgi?id=8941317
It allows switching between unescaped/escaped URL

What do you think?

Honza

Comment 20

[:Harald Kirschner :digitarald]

Please see this attachment: https://bugzilla.mozilla.org/attachment.cgi?id=8941317 It allows switching between unescaped/escaped URL

Looks good. Only concern, shared with Vitoria, is that the use of buttons as toggles and the space they need.

Since then we re-designed the header sections with toggles for Raw Headers. I'd assume we could use the same for the Request URL. For saving space, as we have more Raw ThisField toggles on the same pages; maybe just labeling them with Raw is enough.

For saving more space, the toggle elements seem to take more space than checkboxes and don't have a proper enabled blue state; I wonder if we should switch them out.