-
Notifications
You must be signed in to change notification settings - Fork 22
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
js-based view-source #106
base: master
Are you sure you want to change the base?
js-based view-source #106
Conversation
server.py
Outdated
response.headers['X-Content-Type-Options'] = 'nosniff' | ||
response.headers['X-Frame-Options'] = 'DENY' | ||
response.headers['X-XSS-Protection'] = '1; mode=block' | ||
response.headers['Access-Control-Allow-Origin'] = 'https://hashbang.sh/' |
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.
Why is this needed?
src/scripts.js
Outdated
var link = document.createElement("link"); | ||
link.type = "image/png"; | ||
link.rel = "icon"; | ||
link.href = "data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAABAAAAAQAQAAAAA3iMLMAAAABGdBTUEAALGPC/xhBQAAACBjSFJNAAB6JgAAgIQAAPoAAACA6AAAdTAAAOpgAAA6mAAAF3CculE8AAAAAmJLR0QAAd2KE6QAAAAYSURBVAjXY2CAAck+EKp/B0II9gMoGwYA4+MJkeae/NUAAAAldEVYdGRhdGU6Y3JlYXRlADIwMTQtMDUtMTdUMTM6MzI6MTMtMDQ6MDB7pieOAAAAJXRFWHRkYXRlOm1vZGlmeQAyMDE0LTA1LTE3VDEzOjMxOjQ4LTA0OjAwqyt+rwAAAABJRU5ErkJggg=="; |
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.
If we are moving this around, can't we put it directly in the HTML?
src/scripts.js
Outdated
sourceEl.removeAttribute("href"); | ||
sourceEl.onclick = function(){ | ||
var client = new XMLHttpRequest(); | ||
client.open('GET', 'http://localhost:8080/'); |
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.
NOPE, wrong URL.
(Also, would be forbidden by CSP)
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.
@lrvick I implemented the changes I requested.
Please answer the question regarding the CORS header.
@RyanSquared Please review, since there are now commits I wrote in here.
Changes implemented. One question left.
14effa2
to
526e0f3
Compare
Rebased atop the current |
var client = new XMLHttpRequest(); | ||
client.open('GET', 'https://hashbang.sh/'); | ||
client.onreadystatechange = function() { | ||
console.log(client.responseText) |
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.
debug log left over?
var sourceText = document.createTextNode(client.responseText) | ||
div.appendChild(sourceText) | ||
div.id = "source" | ||
document.getElementsByTagName("html")[0].appendChild(div); |
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.
Instead of document.getElementsByTagName("html")[0]
why not use document.firstElementChild
?
But also, shouldn't you be appending to document.body
anyway?
Do not merge, still some CSS issues to work out (can't scroll).
Overall implementation tested/works.