Elbart Reporter
	Description • 10 years ago

Bug 1121826 reverted the changes of bug 473822 comment 14, and the bugs mentioned in the other comments are happening again, especially the URL-copying of bug 473822 comment 0.

In general, Firefox is handing percent encoded brackets differently, depending on where you look (made with Nightly 01-17):
http://i.imgur.com/ODh0xTK.png
A similar image made with Nighty 01-21 is attached.

Comparison with other browsers (made with Nightly 01-17):
http://i.imgur.com/kqDmRE4.png


Test-URLs:
http://example.org/?foo%5Ba%5D=bar&foo[b]=bar&foo%28c%29=bar&foo(d)=bar
http://example.org/?foo%5Ba%5D=bar
http://example.org/?foo[a]=bar

	Valentin Gosu [:valentin] (he/him)
	Comment 1 • 10 years ago

Interestingly, the dev-tools panel has the same issue as the url-bar. It unescapes URLs.
To illustrate this, open a page that contains console.log(window.location.href);
You will notice, that the page locations are correct, even though the devtools panel says otherwise.
- tested with nightly 38.0a1 (2015-01-22)

I think we need to fix both the devtools issue, and the URL-bar issue.

	Valentin Gosu [:valentin] (he/him)
	Comment 2 • 10 years ago

Hey Victor,
I notice there are several unescape(...) instructions in netmonitor-view.js
Can we get rid of them, or is there a reason they must stay?
IMO the network panel should display the exact URL we are loading.

	Valentin Gosu [:valentin] (he/him)
	Comment 3 • 10 years ago

I think we should also consider if percent decoding in the url-bar is actually necessary:
https://dxr.mozilla.org/mozilla-central/source/browser/base/content/browser.js#2329

I also did a try run to see if removing the decoding would break anything major:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=9a18b6568a66
Only the url-bar decoding tests it seems.

:dao, do you think we could get rid of this decoding? Other browsers don't do it, and at times it is very confusing (for example bug 956463 comment 0)

	Masatoshi Kimura [:emk]
	Comment 4 • 10 years ago

(In reply to Valentin Gosu [:valentin] from comment #3)
> Other browsers don't do it,

Really? At least Chrome decodes non-ascii characters on the Omnibar. Please do not uniformly stop decoding.

	Dão Gottwald [:dao]
	Comment 5 • 10 years ago

(In reply to Valentin Gosu [:valentin] from comment #3)
> :dao, do you think we could get rid of this decoding?

No.

	Victor Porof [:vporof][:vp]
	Comment 6 • 10 years ago

(In reply to Valentin Gosu [:valentin] from comment #2)
> Hey Victor,
> I notice there are several unescape(...) instructions in netmonitor-view.js
> Can we get rid of them, or is there a reason they must stay?
> IMO the network panel should display the exact URL we are loading.

They're there because escaped strings are ugly and hard to read 99% of the time. Is there some exact specific use-case in which this is undesirable? If so, can we special-case it?

	Valentin Gosu [:valentin] (he/him)
	Comment 7 • 10 years ago

(In reply to Victor Porof [:vporof][:vp] from comment #6)
> They're there because escaped strings are ugly and hard to read 99% of the
> time. Is there some exact specific use-case in which this is undesirable? If
> so, can we special-case it?

I think we can. I'll look to see if we already have code that does this, if not, I can write some.

	Valentin Gosu [:valentin] (he/him)
	Comment 8 • 8 years ago

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

	Valentin Gosu [:valentin] (he/him)
	Comment 9 • 8 years ago

(In reply to Valentin Gosu [:valentin] from comment #7)
> (In reply to Victor Porof [:vporof][:vp] from comment #6)
> > They're there because escaped strings are ugly and hard to read 99% of the
> > time. Is there some exact specific use-case in which this is undesirable? If
> > so, can we special-case it?
> 
> I think we can. I'll look to see if we already have code that does this, if
> not, I can write some.

I looked at the problem a bit more, and I don't think we can do this in a sane and consistent way.
Other devtools seem to get away with this, so I don't believe unescaping adds that much value - but it does get in the way in some corner cases.

	Victor Porof [:vporof][:vp]
	Comment 10 • 8 years ago

Comment on attachment 8725309 [details]
MozReview Request: Bug 1124600 - Percent-encoded characters are decoded in Firefox devtools r?vporof

https://reviewboard.mozilla.org/r/37417/#review34399

I don't think we should do this. Seems like a small enough problem with a fix that would not be worth it.

	Valentin Gosu [:valentin] (he/him)
	Comment 11 • 8 years ago

I don't personally see the advantage of keeping things as they are. In the case mentioned in comment 0 it is especially difficult to figure out the actual URL to which we send a request.
Other browsers avoid this problem by not unescaping at all.

Do you think we could change the UX to accommodate this use case? Such as show the real URL by default, and show the unescaped version on mouse-over? This would help both the "can't tell these URLs apart" and "this URL is hard to read" usecases.

Or, if we can't find an adequate solution for this problem, do you think maybe we could add a pref for this?

	Victor Porof [:vporof][:vp]
	Comment 12 • 8 years ago

(In reply to Valentin Gosu [:valentin] from comment #11)
> 
> Do you think we could change the UX to accommodate this use case? Such as
> show the real URL by default, and show the unescaped version on mouse-over?
> This would help both the "can't tell these URLs apart" and "this URL is hard
> to read" usecases.

How about the other way? Show the escaped string by default then the real one on mouse over and other places where you'd click&select to copy.

	Valentin Gosu [:valentin] (he/him)
	Comment 13 • 8 years ago

I guess that's better that what we have at the moment. But I don't know the code well enough to make this happen.
Could you take the bug?

	Valentin Gosu [:valentin] (he/him)
	Comment 14 • 8 years ago

Victor, I'm assigning this to you at the moment, so we don't forget about it.
Feel free to pass it to someone else, or pass it back if you have a better suggestion for a solution. Thanks!

	Victor Porof [:vporof][:vp]
	Comment 15 • 8 years ago

I don't have time to work on this at the moment. I'll look around for a netmonitor maintainer.

	Firefox Bug Husbandry Bot
	Comment 16 • 7 years ago

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258

	Firefox Bug Husbandry Bot
	Comment 17 • 7 years ago

Bulk change to priority: https://bugzilla.mozilla.org/show_bug.cgi?id=1399258