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

Handle percent-escaped hash in target URL fragment #199

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

cl0ne
Copy link
Contributor

@cl0ne cl0ne commented Dec 23, 2020

Proposed workaround for #198.

@lazka
Copy link
Member

lazka commented Dec 24, 2020

Shouldn't

--- a/index.html
+++ b/index.html
@@ -21,7 +21,7 @@ function applyHash() {
         if(!endsWith(hash, ".html") && !endsWith(hash, "/") && hash.indexOf("#", 1) < 0)
             hash += "/";
 
-        new_src = hash.substring(1);
+        new_src = decodeURIComponent(hash.substring(1));
     } else {
         new_src= main_page;
     }

be enough?

@cl0ne
Copy link
Contributor Author

cl0ne commented Dec 24, 2020

I reverted style-related changes.

I've looked at decodeURIComponent earlier, though I'm not sure if it won't break anything by "unescaping too much". So I decided to make the least invasive solution: unescape only the first hash found within fragment part.

Considering the fact that the site consists of static pages only decodeURIComponent should work in most (if not all) cases with a couple of precautions:

--- a/index.html
+++ b/index.html
@@ -12,18 +12,24 @@
 function applyHash() {
     // takes the current hash loads the path into the content frame
 
-    var hash = window.location.hash;
+    var hash = window.location.hash.substring(1);
     var elm = document.getElementById('Content');
-    var new_src;
+    var new_src = main_page;
 
     if(hash) {
+        hash = decodeURIComponent(hash);
+        var fragment_start = hash.indexOf("#");
+        if (fragment_start >= 0) {
+            var new_fragment = hash.substring(fragment_start+1);
+            new_fragment = encodeURIComponent(new_fragment);
+            new_src = encodeURI(hash.substring(0, fragment_start));
+            new_src += "#" + new_fragment
+        } else {
+            // needed for webkit
+            if (!endsWith(hash, ".html") && !endsWith(hash, "/"))
+                hash += "/";
+            new_src = hash;
+        }
-        // needed for webkit
-        if(!endsWith(hash, ".html") && !endsWith(hash, "/") && hash.indexOf("#", 1) < 0)
-            hash += "/";
-
-        new_src = hash.substring(1);
-    } else {
-        new_src= main_page;
     }
 
     // create a dummy element so we can compare the URLs; prevents

@cl0ne
Copy link
Contributor Author

cl0ne commented Jan 19, 2021

So which one of these three solutions should I push for this PR to be accepted?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants