-
Notifications
You must be signed in to change notification settings - Fork 136
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
PoC for deeplinking #161
PoC for deeplinking #161
Conversation
<script> | ||
window.onload = function () { | ||
try { | ||
let params = new URL(document.location.toString()).searchParams; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- And this is where we actually get the params two frames down.
@@ -5,6 +5,22 @@ | |||
<!-- Don't include accord.js --> | |||
</head> | |||
<body> | |||
<script> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And regarding though the menu is generated and checked in
, I guess it would overwrite this PoC code as I guess the code that renders this code is somewhere else...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somewhere here, I guess, the code would have to be generated, if this idea is worth pursuing I could always put it in there
OpenGL-Refpages/es3.0/html/makeindex.py
Line 169 in 9813a55
scriptInclude, |
@@ -4,6 +4,6 @@ | |||
</head> | |||
<frameset rows="160,*"> | |||
<frame scrolling="no" noresize frameborder="0" marginwidth="0" marginheight="0" src="top.php"> | |||
<frame noresize frameborder="0" marginwidth="0" marginheight="0" src="bottom.php"> | |||
<frame noresize frameborder="0" marginwidth="0" marginheight="0" src=<?php echo "bottom.php?" . $_SERVER['QUERY_STRING'] ?> > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1st place the query params must be passed onto the next frame
@@ -3,7 +3,7 @@ | |||
<title>OpenGL ES 3.0 Reference Pages</title> | |||
</head> | |||
<frameset cols="280,*"> | |||
<frame frameborder="0" marginwidth="0" marginheight="0" src="html/indexflat.php"> | |||
<frame frameborder="0" marginwidth="0" marginheight="0" src=<?php echo "html/indexflat.php?" . $_SERVER['QUERY_STRING']?> > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2nd place the query params must be passed onto the next frame
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This probably also calls for heavy sanitation.
let params = new URL(document.location.toString()).searchParams; | ||
let subpage = params.get("subpage"); | ||
if (subpage !== null) { | ||
const links = document.querySelectorAll('a'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By just looking for the link in the DOM and clicking it, we should avoid having to whitelist anything (implicit by existing elements on the page). If it is referring to some link on the page, then the user can click it, if not, then it does not do anything.
} | ||
} | ||
} | ||
} catch (error) { } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very silently failure for this, could log something to the console.
window.onload = function () { | ||
try { | ||
let params = new URL(document.location.toString()).searchParams; | ||
let subpage = params.get("subpage"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
perhaps method
or function
is more precise than subpage.
I think the version with javascript to inject query params calls for unnescessary pageloads, and the one with php sounds scary, as it must be heavy sanitized when running php code with user input, and I don't know php that well. |
As discussed in #160 it would be useful to deeplink into the documentation, for example when wanting to lookup the documentation for a function from neovim as in bjartwolf/nvimconfig@9f1fe75
Locally I did not get the css to load, but it shouldn't matter for this, but the page loads and it should ignore any missing search params or misspelled pages to allow for normal loading.
However, in the current docs with nested frames it isn't obvious how to deeplink into the structure. I tried, and it works, but it isn't pretty. However, something like this could be useful.
I leave it here to discuss.
Just realized, this should likely be done in php, or at least most of it to avoid re-rendering pages. The query parameter is available on the server... IF this is to be done, the server could pick it up and just render the correct page... It is available at the request so it does not have to be sent from the top, and it should also avoid blinking/reload pages to work. Likely, it does not have to go the menu at all, it would just be a question of rendering the correct page and picking that one. However, there are then issues of whitelisting the request etc, I am not sure I know enough php to do this safely.If just passing the query params, unmodified, in php into the last page, then it should not be any issue and the inner JavaScript could pick it up. Passing query params by itself is not an issue, and by not using it on the server to load pages there are a lot of whitelisting and security issues one avoids. It would be possible to serverside render the correct page too, but that opens a whole list of issues with php and user input and security...