1746383 - Restored tabs with HTTPS temporarily show a broken padlock in the address bar

Status: VERIFIED FIXED

(Firefox :: Session Restore, defect)

Description

[Valentin Gosu [:valentin]]

Attachment: Screencast from 16-12-21 13:17:19.webm

This bug was initially reported by @onrefni:mozilla.org in #necko

with fission disabled while loading tabs from previous session no padlock displayed > secure padlock
with fission enabled broken padlock > secure padlock
if i click on broken padlock it says upgraded from http to https by https only mode, happens on every site
if i open new tab and go to same sites it works fine
It seems like a visual bug, but i am a bit worried

I was able to reproduce this on Ubuntu using the following STR:
Close Firefox.
Restore Firefox.
Click on suspended tabs to load them. You will notice the broken padlock appears for less than a second.

Comment 1

[Valentin Gosu [:valentin]]

Note that I'm not seeing the "upgraded from http to https by https only mode" thing when I manage to click the broken padlocks. It just says connection not secure

Comment 2

[:Gijs (away/busy until Mar 10)]

Anny, given this was new with fission, I wonder if you have an idea of what's happening here? The padlock state will be updating based on navigation/history updates it gets from the DOM code, so I expect this is somehow related to either session history in parent or document channel changes, or both...

Comment 3

[:Gijs (away/busy until Mar 10)]

Attachment: Screenshot from onrefni

Comment 4

[:Gijs (away/busy until Mar 10)]

Attachment: Screencast from onrefni - 2021-12-16 16-03-00.mkv

Comment 5

[Alice0775 White]

I can reproduce the issue (when select a pending(discarded) tab)in Firefox91.4esr as well as Nightly97.0a1(20211216094142) Windows10 with/without Fission.

Comment 6

[Alice0775 White]

Regression window :
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf162b065e1feaa64d6df95bc55ee8a894312d34&tochange=ea34fd156c89005e2e8e74c6e6b6663291aa08c8
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fb5ee52cb42c4d4b214ead6d324a0a664b738dcb&tochange=9bcf68d2c0df4b5910ae8f96299a2ff9291720bf

Suspect: 6024bb07171399fe1e8be3cab47afc62079f4cf0 Krystle Salazar — Bug 1570678 - Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh,nhnt11

Comment 7

[Hsin-Yi Tsai (she/her)[:hsinyi]]

(In reply to Alice0775 White from comment #6)

Regression window :
https://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=bf162b065e1feaa64d6df95bc55ee8a894312d34&tochange=ea34fd156c89005e2e8e74c6e6b6663291aa08c8
https://hg.mozilla.org/integration/autoland/pushloghtml?fromchange=fb5ee52cb42c4d4b214ead6d324a0a664b738dcb&tochange=9bcf68d2c0df4b5910ae8f96299a2ff9291720bf

Suspect: 6024bb07171399fe1e8be3cab47afc62079f4cf0 Krystle Salazar — Bug 1570678 - Replace (i) icon for a file icon on potentially trustworthy pages. r=johannh,nhnt11

Thanks for the regression window!
Hi nhnt11, can you please take a look?

Comment 8

[(inactive) Nihanth Subramanya [:nhnt11]]

Forwarding to the privacy/security frontend team.

Comment 9

[Emma Zühlcke [:emz]]

When I run session restore I actually get an invalid page proxy state (search icon shown, no lock icon). I'll put this on my stack of things to look into.

Comment 10

[Emma Zühlcke [:emz]]

I can reproduce the original issue. I think we should set the UrlBar property pageproxystate=invalid until we have the correct identity state to show. Setting this state hides the identity icons and only shows the search icon.

From debugging I can see that we're getting 3 onLocationChange callbacks with different security states. The padlock state is derived from the security stated exposed via gBrowser.securityUI.state. Both the UrlBar state and the site identity state are updated on onLocationChange callbacks.

1. uri: about:blank, isSecure: false. The initial tab switch. This is an artifical onLocationChange callback invoked by the tabbrowser itself.
2. uri: https://example.com, isSecure: false. Called right when the load begins. The state flag is insecure, but the pageproxystate is valid, so we show the broken padlock.

(...) page loads

3. uri: https://example.com, isSecure: true. Called when page content is painted. The state flag is now correct, it shows secure. The broken padlock changes to a regular padlock.

I'm not sure why we get an additional onLocationChange callback here. This does not happen when loading tabs manually.

Comment 11

[Emma Zühlcke [:emz]]

Setting the URI on a restored docShell fires extra onLocationChange and onSecurityChange events.
https://searchfox.org/mozilla-central/rev/69a482382823f42f482e840f65725218d7654cc4/toolkit/components/sessionstore/SessionStoreUtils.cpp#1492
Given that this wouldn't break anything else, we could skip firing onLocationChange here. We'll still get the regular location change when the tab actually starts loading.

Comment 12

[Emma Zühlcke [:emz]]

Attachment: Bug 1746383 - Set URLBar pageProxyState to invalid for onLocationChange triggered by SessionStore. r=nika!,dao!

Comment 13

[Emma Zühlcke [:emz]]

Attachment: Bug 1746383 - Test for identity section security state after tab restore. r=dao!

Depends on D138823

Comment 14

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b07e8132d94a
Set URLBar pageProxyState to invalid for onLocationChange triggered by SessionStore. r=nika,dao
https://hg.mozilla.org/integration/autoland/rev/1556ccfd6acb
Test for identity section security state after tab restore. r=dao

Comment 15

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/b07e8132d94a
https://hg.mozilla.org/mozilla-central/rev/1556ccfd6acb

Comment 16

[BugBot [:suhaib / :marco/ :calixte]]

Set release status flags based on info from the regressing bug 1570678

Comment 17

[Ardelean Oana, Desktop Test Engineering [:oardelean]]

I managed to reproduce this issue on Firefox 100.0(20220428192727) on Ubuntu 20.04. Verified as fixed on Firefox 101.0b4(20220508185621) and Nightly 102.0a1(20220509190429) on Ubuntu 20.04, macOS 11 and Win10 64-bits.