Closed BowlesCR closed 8 years ago
Looks mostly good to me. I am unsure about the HTTPS change; won't this break in environments where HTTPS is unavailable?
If you mean port 443 is blocked in a firewall somewhere, sure... But who does that?
The idea here is that if you are viewing dump1090 via HTTPS (I use Apache as a reverse proxy and SSL terminator) you don't want any image/script references to require HTTP or you will get browser warnings about mixed security content. When viewing via HTTP the browser won't care.
Obviously you can only do this for resources hosted on HTTPS themselves. OpenStreetMap for example doesn't appear to be available that way, or at least not by trivially switching protocols in the same URL.
Also, using //: instead of https:// will cause the browser to use whatever protocol the main page was served with, but I tend to believe you should use HTTPS anywhere available, and the resources are on different servers so why is the main page's protocol relevant.
443 is often blocked in corporate environments.
As a network/infosec engineer, I'd submit that 80 and 443 are both usually as close to wide open as anything gets or pretty much the entire Internet breaks as many sites (Google, Facebook, all US Federal sites, etc) are preferring or requiring HTTPS.
Of course, I'll respect your final vote as the maintainer of the project -- if you'd rather see the protocol-relative :// URLs I'll gladly push another commit to do so.
Yes, please make them protocol-relative. Thanks!
Changes made. I left PlaneFinder as an HTTPS link as they force-redirect serverside.
I also rebased to your master so you don't get a nasty merge conflict with all the whitespace changes.
I'm not going to take the script.js indent changes so this may take a little while to untangle.
Ok, merged except for the script.js indent changes.
I can take a script.js reformat in a separate PR if you can:
Biggest change is using https where available to avoid mixed-content warnings when viewing the page over HTTPS. Additionally tweaked HTML to meet HTML5 specs (it appears that was the original target), and cleaned up a lot of indentation.