-
Notifications
You must be signed in to change notification settings - Fork 102
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
Fix bugs #2360
Fix bugs #2360
Conversation
@@ -39,14 +39,6 @@ export class AppComponent implements OnDestroy { | |||
) {} | |||
|
|||
async ngOnInit() { | |||
if (location.hostname !== 'localhost' && location.protocol === 'http:') { |
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.
are we moving the redirect to the BE? or abandoning it all together?
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.
Abandoning. It's unnecessary and the code was problematic
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.
ok, could you relay why? just curious what changed @dr-bonez? for my own understanding
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.
- It just didn't work in my testing
- If it did work, the UX would be that after a period of time and with no indication why, the page would suddenly redirect/refresh
- There was no catch block, so if the fetch failed, it would be an unhandled promise rejection.
We don't need to force people away from http. We just need to tell them why they should be using https
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.
Cool thanks for explaining. Those concerns could for sure be addressed in code, I definitely made a mistake not adding the catch block. In my testing, it only redirected once on initial load and worked great. Perhaps the lag over Tor would be abrupt. I think we had wanted to do this for a few reasons, rather than making the user manually change, but understood its not ideal to force the behavior.
fix reset tor, delete http redirect, show message for tor http, update release notes