695477 - Use an indicator in toolbar to reflect security state

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect, P1)

Comment 1

[Lucas Rocha (:lucasr)]

Assigning to Julien who's already working on it.

Comment 2

[Julien Vermet | JulienDev]

Attachment: Patch v1

If security state is high, favicon background is blue, if anything else, favicon background is white.

Comment 3

[Julien Vermet | JulienDev]

Attachment: address_bar_button_middle_blue.9.png

address_bar_button_middle_blue.9.png > embedding/android/resources/drawable/address_bar_button_middle_blue.9.png

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

The design has changed. We are not using color to show security level in the toolbar.

Instead, we are showing a lock image. The lock will be hidden if there is no level of security on a site. The lock will appear if the site has some level of security. Tapping the lock will open the security panel (bug 695204).

I'll add a mockup

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: mockup of the lock in the toolbar

Mockup

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: updated patch

This is an update of Julien Vermet's patch in bug 695204. It adds support for storing the security mode in the Tab and updates the security mode in the BrowserToolbar when loading and switching tabs.

Comment 7

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 571800 [details] [diff] [review]
updated patch

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

I see one problem here:
handleSecurityChange() is updating the color without checking if the tabId is for the selectedTab. If a SecurityChange arrives for a background tab, the BrowserToolbar will be updated.

I would suggest using "" instead of "unknown". Also, if they are reserved keywords, it's better to define them as constants and use them.

Comment 8

[Mark Finkle (:mfinkle) (use needinfo?)]

Attachment: updated patch 2

Fixed the selectedTab issue. Leaving "strings" until we move to the next level of this feature, which is bug 695204

Comment 9

[Sriram Ramasubramanian [:sriram]]

Comment on attachment 571803 [details] [diff] [review]
updated patch 2

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

This patch looks fine to me.

Comment 10

[Mark Finkle (:mfinkle) (use needinfo?)]

https://hg.mozilla.org/projects/birch/rev/149e1178490f

Filing a new bug to change the indicator type.

Comment 11

[Aaron Train [:aaronmt]]

(In reply to Mark Finkle (:mfinkle) from comment #10)
> https://hg.mozilla.org/projects/birch/rev/149e1178490f
> 
> Filing a new bug to change the indicator type.

Which?

Comment 12

[Wesley Johnston (:wesj)]

These patches were backed while investigating Talos failures.  Now that tests are green again, we will need to reland.

Comment 13

[Brad Lassey [:blassey] (use needinfo?)]

backout was backed out https://hg.mozilla.org/projects/birch/rev/6f925b45a547

Comment 14

[Aaron Train [:aaronmt]]

20111114041052
http://hg.mozilla.org/projects/birch/rev/859ecdfe0168
Samsung Galaxy SII (Android 2.3.4)

Still waiting for the lock bug mentioned in comment #10