801090 - Work - implement the lock indicator in the Firefox app bar

Status: RESOLVED FIXED

(Firefox for Metro Graveyard :: Browser, defect)

Comment 2

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v1 (WIP)

This patch makes it so that loading an https page causes a "lock" icon to appear.  Loading a regular http page causes the regular "globe" icon to appear.  Also, typing in the URL bar after loading an https page will cause the lock to turn back into a globe.

Still to do:
  Make it so that switching tabs works properly (the lock icon should stay on the tabs that caused it to appear)

Questions:
  - There are some icons in [1] that seem like the icons we intend to use (e.g. identity-icons-https.png), but those icons are sprites and don't seem to be the right size (they're not the same size as the globe icon we use for regular browsing) so I'm not sure which icons I should be using.  I'm currently using locked-hdpi.png and unlocked-hdpi.png.
  - Do we want to implement the whole security panel (i.e. what appears when you click on the lock/globe), or do we just want the icon to appear?

[1] https://mxr.mozilla.org/mozilla-central/source/browser/metro/theme/images/

Comment 3

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Note; the patch for this bug only works after applying the patch for bug 771348.

Comment 4

[Marco Mucci [:MarcoM]]

Hi Stephen, please see Tim's question in Comment #2.

Comment 5

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v2

This patch updates WebProgress.js to store identity information per tab.
  - When a SecurityChange event is received, the identity state for that tab is saved
  - When the URL bar is typed into, the identity state information for that tab is invalidated
  - When the current tab is switched, the identity state for the selected tab is loaded

Some CSS decides which icon (lock or globe) to display based on the identity information for the selected tab.

This patch doesn't use the correct icons, but we can easily fix that later when the correct icons are available.

Comment 6

[Matt Brubeck (:mbrubeck)]

Comment on attachment 750189 [details] [diff] [review]
Patch v2

Review of attachment 750189 [details] [diff] [review]:
-----------------------------------------------------------------

::: browser/metro/base/content/WebProgress.js
@@ +18,5 @@
>      Elements.progress.addEventListener("transitionend", this._progressTransEnd, true);
> +    Elements.tabList.addEventListener("TabSelect", this._onTabSelect, true);
> +
> +    let urlBar = document.getElementById("urlbar-edit");
> +    urlBar.addEventListener("input", this._onUrlBarInput, false);    

nit: trailing whitespace

@@ +69,3 @@
>  
>      aTab.hostChanged = false;
> +    aTab.state = state;

Is tab.state actually used anywhere?  If not, remove it.

@@ +220,5 @@
> +    identityBox.className = tab._identityState || "";
> +  },
> +
> +  _onUrlBarInput: function(aEvent) {
> +    let identityBox = document.getElementById("identity-box-inner");      

trailing whitespace

Comment 7

[Matt Brubeck (:mbrubeck)]

Comment on attachment 750189 [details] [diff] [review]
Patch v2

>-    // Don't need to do anything if the data we use to update the UI hasn't changed
>-    if (aTab.state == aJson.state && !aTab.hostChanged)
>-      return;

Ah, here's where that stuff was used.  Looks like we no longer need .state or .hostChanged, and we can also remove them from the Tab constructor in browser.js.

Comment 8

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

Attachment: Patch v3

Review comments addressed

Comment 9

[Tim Abraldes [:TimAbraldes] [:tabraldes]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/1428db8c9f10

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/1428db8c9f10