Firefox escapes single quote when accessing window.location properties via javascript

RESOLVED FIXED in Firefox 57

Status

()

defect
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: ddumont, Assigned: valentin)

Tracking

(Blocks 1 bug, {dev-doc-complete, site-compat})

54 Branch
mozilla57
Points:
---

Firefox Tracking Flags

(firefox57 fixed)

Details

(Whiteboard: [necko-active])

Attachments

(1 attachment)

(Reporter)

Description

2 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.
See also Bug 1184589
Component: Untriaged → Networking
Product: Firefox → Core
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?
Flags: needinfo?(annevk)
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.

Comment 5

2 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.
Blocks: url
Flags: needinfo?(annevk)
@valentin might know better about what's our next move.
Flags: needinfo?(valentin.gosu)
Should be easy enough to fix.
Assignee: nobody → valentin.gosu
Flags: needinfo?(valentin.gosu)
Whiteboard: [necko-active]
Comment hidden (mozreview-request)
(Reporter)

Comment 10

2 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?

Comment 11

2 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
Attachment #8894200 - Flags: review?(juhsu) → review+

Comment 12

2 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

Comment 13

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/bcf82372275b
Status: UNCONFIRMED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla57
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.
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.
(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!
(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.
(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!
(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.
You need to log in before you can comment on or make changes to this bug.