Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: rewrite URLs so that %3D chars are rendered as = #539

Closed
erikmd opened this issue Apr 19, 2023 · 3 comments · Fixed by #557, #511 or ocaml/opam-repository#24305
Closed

Feature: rewrite URLs so that %3D chars are rendered as = #539

erikmd opened this issue Apr 19, 2023 · 3 comments · Fixed by #557, #511 or ocaml/opam-repository#24305
Assignees
Labels
issue: upstream Issue handled upstream kind: enhancement Enhancement to an existing user-facing feature.

Comments

@erikmd
Copy link
Member

erikmd commented Apr 19, 2023

Related user(s):

@erikmd

Related issue(s) or PR(s):

No response

Related project scope(s):

web-app UI

The problem:

When opening an exercise in a dynamic learn-ocaml server, the = chars are unexpectedly rendered as %3D:

Wanted solution:

It would be nice to keep the = chars as is thanks to URL rewriting (if possible):

Considered alternatives:

No response

Additional context:

No response

@erikmd erikmd added kind: feature New user-facing feature. kind: enhancement Enhancement to an existing user-facing feature. and removed kind: feature New user-facing feature. labels Apr 19, 2023
@Plictox
Copy link
Collaborator

Plictox commented May 12, 2023

If you agree I think I can do the same thing for the '&' character, as in this URL : http://localhost:8080/exercises/mooc/week1/seq3/ex1/#tab%3Dtext%26prelude%3Dshown

@erikmd
Copy link
Member Author

erikmd commented May 12, 2023

@Plictox yes! that looks good.

@erikmd erikmd assigned erikmd and AltGr and unassigned Plictox May 16, 2023
@erikmd
Copy link
Member Author

erikmd commented May 16, 2023

@Plictox FYI I did a few searches on MDN, and I have some news:

  1. The culprit in learn-ocaml's code base is this code:

let set_fragment args =
let pairs = List.map (fun (n, v) -> n ^ "=" ^ v) args in
let fragment = String.concat "&" pairs in
Url.Current.set_fragment fragment

  1. Replacing Url.Current.set_fragment fragment with Dom_html.window##.location##.hash := Js.bytestring fragment solves the issue.

  2. FTR, the location.hash is documented at https://developer.mozilla.org/en-US/docs/Web/API/Location/hash

  3. There was an old bug in Firefox w.r.t. encoding/decoding only for the getter (see e.g. https://stackoverflow.com/questions/1703552/encoding-of-window-location-hash ), but AFAICT, it is unneeded to manually url-encode also the URLs for the setter.

So I assign @AltGr and myself for now, we'll see at our next maintainer telco (end-May) if we propose a patch upstream to simplify Url.Current.set_fragment

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue: upstream Issue handled upstream kind: enhancement Enhancement to an existing user-facing feature.
Projects
None yet
3 participants