lock/unlock icon disappears when downloading from a site

[LaurenWags]

Description

When downloading (ex. from github.com/brave/browser-laptop/releases) the lock/unlock icon disappears from the URL bar.

Steps to Reproduce

1. Navigate to github.com/brave/browser-laptop/releases
2. Click on a release to download (if you're quick, you can see the lock icon missing before the save dialog comes up)
3. Once release starts downloading, look at the URL bar.

Actual result:
Lock icon disappears

Expected result:
Lock icon should remain

Reproduces how often:
Easily

Brave Version

about:brave info:

Brave | 0.19.113
rev | 043c506
Muon | 4.5.25

Reproducible on current live release:
Yes, reproduced on 0.19.105

Additional Information

[bsclifton]

Related to #12177?
cc: @darkdh

edit: nevermind- I see it's also reproduced on 0.19.105 😄

[MargarytaChepiga]

I really want to work on this issue. As I can see it was fixed before for Muon version 4.5.24 (or it actually wasn't?) :
brave/muon@cdeecd9
I will dive into it, and see what I can do. I would really appreciate any suggestions though :)

[MargarytaChepiga]

So I noticed that this doesn't happen when the connection is insecure:

This gives me several ideas:

• It is handled for the insecure connection, but is not handled for secure
• It is handled for both, but the secure just doesn't work

I am trying to debug to find where exactly this process is happening. So far I think it has something to do with window state and/or download item files. Not sure if I am going the right direction.

[NejcZdovc]

@MargarytaChepiga let me check this issue quickly and will let you know asap

[MargarytaChepiga]

Hello! I found two ways to fix this issue. Both of them work, but I am not sure if that's a correct way of fixing this issue.
First, in this file:

browser-laptop/js/stores/windowStore.js

Line 312 in fbb45d2

windowState = windowState.mergeIn(statePath('frames').concat(['security']), {

If I will comment out lines from 314 till 316:

The result will be:

I checked if it works on:

• Secure website with secure download
• Insecure website with an insecure download link
• Secure website with an insecure download link
• Twitter
• PDF downloads from both secure and insecure websites

But I noticed that those three lines were added on purpose in order to reset security state.
Here is the commit with the change made by @diracdeltas : 7990e04#diff-733d30fbf34816322d3f25dd59c1eb99R317

The thing is when we click on download those lines of code executes and resets the security state, so isSecure becomes null etc. According to the tests though this is the expected behavior. When I comment out those three lines everything works perfectly. I also check the test plan for this since those lines were a part of the fix. Everything seems to work perfect and nothing have broken so far. However, I am not fully sure why do we need those lines but I have a strong feeling that we need them. @diracdeltas, I would really appreciate your help here.

Since I am assuming those three lines are important and I shouldn't remove them, I found another solution. As I already mention windowState resets isSecure to null. And the reason why icon disappears is that we don't handle the situation when isSecure is null.
So another solution could be in this file;

browser-laptop/app/renderer/components/navigation/urlBarIcon.js

Line 37 in fbb45d2

get iconCssClasses () {

Here:

If we add the check for null:

However, this solution is not perfect too. Since the state get's reset, the certificate becomes undefined and disappears.

Another thing that I have noticed is that icon and certificate are not disappearing when we want to download pdf, so I think that this is handled for the pdf. However, I am still trying to figure out where and how. Any help and suggestions are highly appreciated!

[diracdeltas]

@MargarytaChepiga thanks for the detailed report! I think the right solution is probably "don't reset the security state if the windowConstants.WINDOW_WEBVIEW_LOAD_START event corresponds to a download event". that is, add an if condition in

browser-laptop/js/stores/windowStore.js

Line 309 in fbb45d2

// Reset security state

that checks whether we are starting a download. i don't know offhand how to do this check, but i bet that it is possible.

you can start by console.logging what the action is for a download; if there isn't enough information in there, you can try adding more properties to it.

[MargarytaChepiga]

Thank you so much @diracdeltas! That was very helpful!!!

[srirambv]

This happens when a torrent download is cancelled in a tor private tab.

[bsclifton]

Closing per #13541 (comment)