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

Added implicit https scheme patch #628

Merged
merged 4 commits into from Dec 14, 2018

Conversation

Projects
None yet
4 participants
@tie
Copy link
Contributor

tie commented Dec 14, 2018

All URLs entered in omnibox without explicit scheme will have https:// prefix instead of http://. That is, HTTPS by default without any extension.
This feature is not configurable because plain-text requests directly affect user's privacy.

Also, it could be my imagination, but looks like Chromium loads web pages significantly faster with this patch.

As shown below, some sites do not redirect to HTTPS by default.

Before

vanilla

After

patched

Alternatives

  • https-by-default uses similar approach for Chromium, but patches omnibox autocomplete instead of the URL fixer.

  • HTTPS Everywhere, etc.

    • Hard to setup if (e.g. in my case Chromium runs from tmpfs).
    • Third-party code (which probably has performance issues) running in the browser.
    • Usually just a database of sites known to support HTTPS.
  • Entering URL with explicit scheme manually.

    Inconvenient and error-prone.

@tie tie changed the title Added implicit https scheme patch [WIP] Added implicit https scheme patch Dec 14, 2018

@tie

This comment has been minimized.

Copy link
Contributor Author

tie commented Dec 14, 2018

I've fixed suggestions in the last commit. Scheme is trimmed in suggestion dropdown for URLs without explicit scheme.

@tie tie changed the title [WIP] Added implicit https scheme patch Added implicit https scheme patch Dec 14, 2018

@tie

This comment has been minimized.

Copy link
Contributor Author

tie commented Dec 14, 2018

@Eloston, should be ready for merging. What's your opinion?

@Eloston

This comment has been minimized.

Copy link
Owner

Eloston commented Dec 14, 2018

These changes are well done. However, will this patch affect other ways of loading web pages? For example, how about command-line-supplied web addresses?

@tie

This comment has been minimized.

Copy link
Contributor Author

tie commented Dec 14, 2018

However, will this patch affect other ways of loading web pages?

Thanks for noticing that.

I've looked through the users of FixupURL on GitHub chromium mirror and, except omnibox (which is already covered by this patch), only searchbox.cc depends on old FixupURL implementation.

Also, url_formatter::FixupURL is used in most of the user URL input parsing, so this should affect command-line usage too.

I'll describe the behavior when I come home because I don't have the build right now.

@saltama

This comment has been minimized.

Copy link
Contributor

saltama commented Dec 14, 2018

As long time user of https everywhere, great patch, thanks.

@tie

This comment has been minimized.

Copy link
Contributor Author

tie commented Dec 14, 2018

With the last commit URLs without explicit scheme entered in chrome://bookmarks will be prefixed with https://.

Command line usage works as expected — chromium example.com opens a new tab for https://example.com URL.

@Eloston

This comment has been minimized.

Copy link
Owner

Eloston commented Dec 14, 2018

Awesome. Thanks for looking into it.

@Eloston Eloston merged commit ab52108 into Eloston:master Dec 14, 2018

@Eloston Eloston added the enhancement label Dec 14, 2018

@Eloston Eloston added this to the 71.x.x.x milestone Dec 14, 2018

@tie tie deleted the tie:https-scheme branch Dec 15, 2018

@intika

This comment has been minimized.

Copy link
Contributor

intika commented Dec 29, 2018

Nice patch 👍 but should not this added with a flag or or something there is a lot site using only http, i know that most of them will be opened after searching a web-engine... any way just my 2 cents not really important

@Eloston

This comment has been minimized.

Copy link
Owner

Eloston commented Dec 29, 2018

@intika That isn't affected by this patch. Only URLs without a scheme are considered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.