Last Comment Bug 695477 - Use an indicator in toolbar to reflect security state
: Use an indicator in toolbar to reflect security state
[birch] [ux needed][QA+]
: feature
Product: Firefox for Android
Classification: Client Software
Component: General (show other bugs)
: unspecified
: ARM Android
: P1 normal (vote)
: ---
Assigned To: Julien Vermet | JulienDev
: Sebastian Kaspari (:sebastian)
: 705339 (view as bug list)
Depends on:
Blocks: 695204
  Show dependency treegraph
Reported: 2011-10-18 13:04 PDT by Erin Lancaster [:elan]
Modified: 2016-07-29 14:20 PDT (History)
9 users (show)
See Also:
Crash Signature:
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---

Patch v1 (3.80 KB, patch)
2011-10-27 06:04 PDT, Julien Vermet | JulienDev
no flags Details | Diff | Splinter Review
address_bar_button_middle_blue.9.png (3.07 KB, image/png)
2011-10-27 06:05 PDT, Julien Vermet | JulienDev
no flags Details
mockup of the lock in the toolbar (20.16 KB, image/png)
2011-10-27 12:23 PDT, Mark Finkle (:mfinkle) (use needinfo?)
no flags Details
updated patch (13.87 KB, patch)
2011-11-03 15:21 PDT, Mark Finkle (:mfinkle) (use needinfo?)
sriram.mozilla: review-
Details | Diff | Splinter Review
updated patch 2 (14.03 KB, patch)
2011-11-03 15:34 PDT, Mark Finkle (:mfinkle) (use needinfo?)
sriram.mozilla: review+
Details | Diff | Splinter Review

Description Erin Lancaster [:elan] 2011-10-18 13:04:06 PDT

Comment 1 Lucas Rocha (:lucasr) 2011-10-27 02:50:01 PDT
Assigning to Julien who's already working on it.
Comment 2 Julien Vermet | JulienDev 2011-10-27 06:04:45 PDT
Created attachment 569938 [details] [diff] [review]
Patch v1

If security state is high, favicon background is blue, if anything else, favicon background is white.
Comment 3 Julien Vermet | JulienDev 2011-10-27 06:05:49 PDT
Created attachment 569939 [details]

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?) 2011-10-27 12:22:38 PDT
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?) 2011-10-27 12:23:25 PDT
Created attachment 570049 [details]
mockup of the lock in the toolbar

Comment 6 Mark Finkle (:mfinkle) (use needinfo?) 2011-11-03 15:21:57 PDT
Created attachment 571800 [details] [diff] [review]
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] 2011-11-03 15:27:25 PDT
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?) 2011-11-03 15:34:53 PDT
Created attachment 571803 [details] [diff] [review]
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] 2011-11-03 15:47:42 PDT
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?) 2011-11-03 15:55:29 PDT

Filing a new bug to change the indicator type.
Comment 11 Aaron Train [:aaronmt] 2011-11-08 06:20:16 PST
(In reply to Mark Finkle (:mfinkle) from comment #10)
> Filing a new bug to change the indicator type.

Comment 12 Wesley Johnston (:wesj) 2011-11-10 10:26:11 PST
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?) 2011-11-11 08:46:59 PST
backout was backed out
Comment 14 Aaron Train [:aaronmt] 2011-11-14 06:42:36 PST
Samsung Galaxy SII (Android 2.3.4)

Still waiting for the lock bug mentioned in comment #10
Comment 15 Aaron Train [:aaronmt] 2011-11-25 10:41:07 PST
*** Bug 705339 has been marked as a duplicate of this bug. ***

Note You need to log in before you can comment on or make changes to this bug.