Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Don't show HTTP icon while site is loading (since it may be HTTPS) #5498

Merged
merged 1 commit into from
Nov 9, 2016

Conversation

bsclifton
Copy link
Member

  • Submitted a ticket for my issue if one did not already exist.
  • Used Github auto-closing keywords in the commit message.
  • Added/updated tests for this change (for new code or code which already has tests).
  • Ran git rebase -i to squash commits (if needed).

Don't show HTTP icon while site is loading (since it may be HTTPS)

Fixes #5490

Includes breaking the urlbar icon into it's own control (along with click/drag events)

Auditors: @diracdeltas, @jkup, @bbondy

Test Plan:

  1. Launch Brave and open a new tab
  2. Type in "https://twitter.com" and get really close to your screen
  3. Hit enter and notice you do not see the yellow triangle at any time

Fixes #5490

Includes breaking the urlbar icon into it's own control (along with click/drag events)

Auditors: @jkup, @bbondy

Test Plan:
1. Launch Brave and open a new tab
2. Type in "https://twitter.com" and get really close to your screen
3. Hit enter and notice you do not see the yellow triangle at any time
@diracdeltas
Copy link
Member

diracdeltas commented Nov 9, 2016

i still see the insecure icon flash briefly. possibly before this.props.loading is set to true, and it's showing the security state of the current page instead of the page that is about to be loaded.

how about, after the 'enter' key is pressed, show no icon at all until the load is started?

@bsclifton
Copy link
Member Author

bsclifton commented Nov 9, 2016

edit: repro'd

@diracdeltas
Copy link
Member

diracdeltas commented Nov 9, 2016

@bsclifton i went to the https url directly but it still showed the insecure icon. i think if one goes to the insecure version, it should show the insecure icon because the browser does in fact make an insecure network connection.

@diracdeltas
Copy link
Member

diracdeltas commented Nov 9, 2016

I'm going to merge this for now since it mitigates the issue but i'll keep the issue open and create another PR on top of it using brave/muon#90

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Insecure icon briefly displayed even for https sites wrongly
6 participants