ddumont Reporter
	Description • 7 years ago

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/60.0.3112.78 Safari/537.36

Steps to reproduce:

Go to google.com#foo'bar


Actual results:

URL looks fine, open debugger and eval window.location.hash

You get "#foo%27bar"


Expected results:

it should have been #foo'bar

Ref: Bug 751411  (should have never been closed against other bugs as dupe, as this was fixed but only in https://bugzilla.mozilla.org/show_bug.cgi?id=1040285 and only for what's sent over the wire... the javascript bug remains)

Please either re-open 751411 and close this one, or use this bug to track the issue and fix the javascript problem.

The most notable fallout of this bug is frameworks that provide router capabilities fail to recognize oldroute/newroute when navigating due to encoding changes.

Ref: https://github.com/jashkenas/backbone/issues/2523
Note: This issue also exists in Dojo's router.

	Kohei Yoshino [:kohei]
	Comment 1 • 7 years ago

See also Bug 1184589

	Kohei Yoshino [:kohei]
	Comment 2 • 7 years ago

I meant Bug 1093611.

	Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)
	Comment 3 • 7 years ago

Test with latest Chrome/Edge/Safari, all three browsers shows "#foo'bar" in developer console when typing |window.location.hash|.

@annevk, what does the latest spec say about whether we should decode the window.location.hash?

	Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)
	Comment 4 • 7 years ago

BTW, preference "dom.url.getters_decode_hash" is removed in bug 1342438 so there is no way to config the behavior as bug 1184589 comment #5 said.

	Anne (:annevk)
	Comment 5 • 7 years ago

1. I don't think there's any URL API that does any kind of decoding. That's all taken care of at parse time. See https://html.spec.whatwg.org/multipage/history.html#dom-location-hash for location.hash.
2. The parser does not escape ' (only C0 and non-ASCII get escaped) per https://url.spec.whatwg.org/#fragment-state.

	Shih-Chiang Chien [:schien] (UTC+8) (use ni? plz)
	Comment 6 • 7 years ago

@valentin might know better about what's our next move.

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

Should be easy enough to fix.

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

https://treeherder.mozilla.org/#/jobs?repo=try&revision=9ac49602aa5e99c7ca7e49af2100d8ad4bb01017

	ddumont Reporter
	Comment 10 • 7 years ago

Thank you for the attention on this bug. Would it be possible to get it back-ported to the ESR so that our customers who use that can get the fix as well?

	Junior [inactive]
	Comment 11 • 7 years ago
mozreview-review

Comment on attachment 8894200 [details]
Bug 1386683 - Do not escape the `'` character in the URL hash

https://reviewboard.mozilla.org/r/165292/#review171982

	Pulsebot
	Comment 12 • 7 years ago

Pushed by valentin.gosu@gmail.com:
https://hg.mozilla.org/integration/autoland/rev/bcf82372275b
Do not escape the `'` character in the URL hash r=junior

	Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)
	Comment 13 • 7 years ago
bugherder

https://hg.mozilla.org/mozilla-central/rev/bcf82372275b

	Oriol Brufau [:Oriol]
	Comment 14 • 7 years ago

The problem affects various other things than single quotes, see bug 1329960.
The proper way to fix this would be using esc_Ref|esc_OnlyNonASCII flags instead of altering EscapeChars.

	Chris Mills [:cmills]
	Comment 15 • 7 years ago

To document this, I've added a note to the window.location browser compat data:

https://developer.mozilla.org/en-US/docs/Web/API/Window/location#Browser_compatibility

Let me know if this looks OK. I didn't add a note to the rel notes, as they are more for new features rather than bug fixes.

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

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #15)
> To document this, I've added a note to the window.location browser compat
> data:
> 
> https://developer.mozilla.org/en-US/docs/Web/API/Window/
> location#Browser_compatibility
> 
> Let me know if this looks OK. I didn't add a note to the rel notes, as they
> are more for new features rather than bug fixes.

The bug applied to all URL APIs - not just window.location
Other than that the description looks good, thanks!

	Chris Mills [:cmills]
	Comment 17 • 7 years ago

(In reply to Valentin Gosu [:valentin] from comment #16)
> (In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #15)
> > To document this, I've added a note to the window.location browser compat
> > data:
> > 
> > https://developer.mozilla.org/en-US/docs/Web/API/Window/
> > location#Browser_compatibility
> > 
> > Let me know if this looks OK. I didn't add a note to the rel notes, as they
> > are more for new features rather than bug fixes.
> 
> The bug applied to all URL APIs - not just window.location
> Other than that the description looks good, thanks!

Ah, ok, I understand now. So it would affect stuff like 

https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
https://developer.mozilla.org/en-US/docs/Web/API/URL
https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils
https://developer.mozilla.org/en-US/docs/Web/API/URLUtilsReadOnly

What else?

If there is a lot of stuff, I might just put a more general note somewhere else.

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

(In reply to Chris Mills (Mozilla, MDN editor) [:cmills] from comment #17)
> > The bug applied to all URL APIs - not just window.location
> > Other than that the description looks good, thanks!
> 
> Ah, ok, I understand now. So it would affect stuff like 
> 
> https://developer.mozilla.org/en-US/docs/Web/API/URLSearchParams
> https://developer.mozilla.org/en-US/docs/Web/API/URL
> https://developer.mozilla.org/en-US/docs/Web/API/HTMLHyperlinkElementUtils
> https://developer.mozilla.org/en-US/docs/Web/API/URLUtilsReadOnly
> 
> What else?
> 
> If there is a lot of stuff, I might just put a more general note somewhere
> else.

Apart from window.location and the above, I am not aware of any others.
Thanks!

	Chris Mills [:cmills]
	Comment 19 • 7 years ago

(In reply to Valentin Gosu [:valentin] from comment #18)
 
> Apart from window.location and the above, I am not aware of any others.
> Thanks!

Cool, thanks Valentin. I've added the note to those other pages too.

	Kohei Yoshino [:kohei]
	Comment 20 • 7 years ago

Posted the site compatibility doc: https://www.fxsitecompat.com/en-CA/docs/2017/location-hash-no-longer-escapes-single-quotes/