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

Specify maximum timeout duration #7

Closed
Gitoffthelawn opened this issue Apr 1, 2019 · 11 comments
Closed

Specify maximum timeout duration #7

Gitoffthelawn opened this issue Apr 1, 2019 · 11 comments
Labels
enhancement New feature or request feedback wanted ux As in any of: usability, accessibility, visual design, interaction design
Milestone

Comments

@Gitoffthelawn
Copy link

Would it be possible to specify the maximum time duration to wait (the first time, or after a reset) before declaring that a site does not support HTTPS?

It would possibly not be 100% accurate to specify a shorter timeout, but it would greatly increase usability.

For example, sites like http://recordit.co/ take a long time to timeout when attempting HTTPS connections.

@Gitoffthelawn
Copy link
Author

BTW, here is an example of a Web Extension that was able to adjust the timeout for non-responsive URL's: cadeyrn/bookmarks-organizer#23 (comment)

@claustromaniac
Copy link
Owner

I don't think I can make a reliable implementation of this without keeping track of tabs and so, which would introduce compatibility issues with other extensions that redirect requests or reopen tabs (like the ones for containers). A workaround would be to lower the values of the relevant settings in about:config, like network.http.connection-timeout and network.http.tls-handshake-timeout

@Gitoffthelawn
Copy link
Author

That's too bad, but I completely understand.

@claustromaniac
Copy link
Owner

claustromaniac commented May 18, 2019

I tried this idea, and it turns out it too introduces compatibility issues.

I think I'm going to allow users to set a maximum timeout duration (as asked here), but this functionality will be disabled by default.

It will take some more time though. I'm quite busy and I have some other priorities anyway.

@Gitoffthelawn
Copy link
Author

Thanks. Take your time.

BTW, when you mention "compatibility issues", what do you mean?

@claustromaniac
Copy link
Owner

When it comes to this specific issue, I'm mostly thinking of this extension triggering other extensions that work with tabs, in situations when it shouldn't. It is mostly about unexpected behavior (e.g. triggering extensions and causing them to open multiple new tabs when they shouldn't), but there can be more serious cases, depending on what the other extensions are meant to do.

Implementing this feature should be a relatively simple task, but there are no webExtensions APIs for redirecting or aborting requests after a certain period of time, so I have to resort to hacky approaches that rely on alternate circumstantial factors that aren't as predictable/consistent (such as tabs, since other extensions can and will replace them sometimes and there is no way for this extension to appropriately react to that). This means that such code by design would not get along with some other extensions.

I want to avoid as many potential conflicts as possible, because I prefer to write shit that works well universally from the start, instead of "fixing" and refactoring the code as people report conflicts. Also, I'd rather save myself the hassle of adding more code complexity, since that often brings along bugs and gradually makes projects harder to maintain. Regardless, when conflicts can't be avoided (as in this case), I can always add the functionality as an option, and let people figure out themselves the reason it comes disabled by default.

@Gitoffthelawn
Copy link
Author

I agree with everything you wrote 💯%.

Which has more compatibility issues: setting a timeout, or putting in the "intermediary" page like you proposed?

BTW, I think full compatibility with Temporary Containers will be important.

@claustromaniac
Copy link
Owner

Which has more compatibility issues... that's hard to say. A better question would be: which would you prefer if they were both more or less equally imperfect?

I don't use many extensions myself, but I obviously do use TC and it is incidentally the best benchmark I can think of for this. When I can, I'm gonna rewrite part of the extension and then I'll see how this turns out. Don't cross your fingers. 😅

@Gitoffthelawn
Copy link
Author

I'll give it some thought. Thanks!

@claustromaniac claustromaniac added this to the 0.8.0 milestone Jun 3, 2019
@claustromaniac
Copy link
Owner

claustromaniac commented Jun 3, 2019

I added this option in 0.8.0b4. As I said previously, it's not perfect. Also, I barely tested it, so there might be issues I'm not aware of yet.

I'm not very convinced about the description of this new feature in the options page... I'm thinking it may not sound natural to native english speakers. Suggestions are welcome.

@Gitoffthelawn
Copy link
Author

The new string reads well. 👍

@claustromaniac claustromaniac added the ux As in any of: usability, accessibility, visual design, interaction design label Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request feedback wanted ux As in any of: usability, accessibility, visual design, interaction design
Projects
None yet
Development

No branches or pull requests

2 participants