[CustomTab] 3 different types of displaying site info in URL bar should be designed in custom tab while there is only one type in chrome. In firefox browser we, 3 different types of displaying site info in URL Bar is possible. refer to the attachment .

chrome has only one type of site info

Firefox has 3 types of site info in URL Bar- Type1

Firefox has 3 types of site info in URL Bar- Type2

Firefox has 3 types of site info in URL Bar- Type3

Hi Jack, would you please kindly provide the UX spec for the reference of engineering efforts ? Thank you !!

Please refer to the attachment. The attached is mockup and our visual designer is working on redesign them. Thank you Jack

Hi Julian, is the UX spec in comment6 enough for engineering team to work on this ? Thank you very much !

The draft is enough to me to survey how to implement it.

I don't really know this code, so I think :sebastian should review these patches.

Jim: Understood, I changed the reviewer to Sebastian, thanks!

Comment on attachment 8847001 [details] Bug 1332546 - Refactory ActionBar by extracting methods to another class https://reviewboard.mozilla.org/r/120038/#review122486 Awesome. That's pretty nice! ::: commit-message-6d38a:1 (Diff revision 1) > +Bug 1332546 - Refactory ActionBar nit: The title of the commit could be a bit more descriptive :)

Comment on attachment 8847002 [details] Bug 1332546 - ensure ActionBar color is not translucent https://reviewboard.mozilla.org/r/120040/#review122488 Are there some apps that set a translucent color? What does Chrome do?

Comment on attachment 8847003 [details] Bug 1332546 - Apply default actionbar color https://reviewboard.mozilla.org/r/120042/#review122490

Comment on attachment 8847004 [details] Bug 1332546 - Add CustomView to ActionBar of CustomTabsActivity https://reviewboard.mozilla.org/r/120044/#review122492 ::: mobile/android/base/java/org/mozilla/gecko/toolbar/SecurityModeUtil.java:22 (Diff revision 1) > + > +/** > + * Util class which encapsulate logic of how CustomTabsActivity treats SiteIdentity. > + * TODO: Bug 1347037 - This class should be reusable for other components > + */ > +public class SecurityModeUtil { This looks like a great class for some unit tests. Also: Let's file a follow-up bug to use this in BrowserApp too. Fragmenting the logic will bite us as soon as we make changes. ::: mobile/android/base/java/org/mozilla/gecko/toolbar/SecurityModeUtil.java:75 (Diff revision 1) > + ? securityModeMap.get(securityMode) > + : Mode.UNKNOWN; > + } > + > + // Security mode constants, which map to the icons / levels defined in: > + // http://dxr.mozilla.org/mozilla-central/source/mobile/android/base/java/org/mozilla/gecko/resources/drawable/customtabs_site_security_level.xml This URL returns a 404 error. On the right side of MXR you can find a permalink that will always link to the version you are looking at and not change when the repository changes. ::: mobile/android/base/resources/drawable/customtabs_site_security_icon.xml:6 (Diff revision 1) > +<level-list xmlns:android="http://schemas.android.com/apk/res/android"> > + > + <item > + android:drawable="@drawable/site_security_unknown" > + android:maxLevel="0"/> Isn't this list the same as site_security_level.xml?

Comment on attachment 8847005 [details] Bug 1332546 - add initialization checking to SiteIdentityPopup https://reviewboard.mozilla.org/r/120046/#review122530 ::: commit-message-39ec9:1 (Diff revision 1) > +Bug 1332546 - add initialization checking to SiteIdentityPopop Popop :)

Comment on attachment 8847004 [details] Bug 1332546 - Add CustomView to ActionBar of CustomTabsActivity https://reviewboard.mozilla.org/r/120044/#review122492 > This looks like a great class for some unit tests. > > Also: Let's file a follow-up bug to use this in BrowserApp too. Fragmenting the logic will bite us as soon as we make changes. Agree :D I would like to write a test for this class, however so far I don't have enough clue(spec) to do this. I created bug #1347037 as the follow-up bug. (Maybe title of the bug is not proper one?) > This URL returns a 404 error. On the right side of MXR you can find a permalink that will always link to the version you are looking at and not change when the repository changes. 404 due to this patch haven't not landed. I think once #1347037 complete, this (for custom tabs) drawble should be removed, so I just use this link for now. > Isn't this list the same as site_security_level.xml? Similiar but a bit different. For example, in site_security_level.xml, maxLevel 1 and 2 refer to same drawable. And this file should be removed due to bug #1347037

Comment on attachment 8847006 [details] Bug 1332546 - Add SiteIdentityPopup to CustomTabsActivity https://reviewboard.mozilla.org/r/120048/#review123418

Pushed by ryanvm@gmail.com: https://hg.mozilla.org/integration/autoland/rev/b65a8df84b11 Refactory ActionBar by extracting methods to another class r=sebastian https://hg.mozilla.org/integration/autoland/rev/b296196d1859 ensure ActionBar color is not translucent r=sebastian https://hg.mozilla.org/integration/autoland/rev/4796c44dda66 Apply default actionbar color r=sebastian https://hg.mozilla.org/integration/autoland/rev/540020bee3a6 Add CustomView to ActionBar of CustomTabsActivity r=sebastian https://hg.mozilla.org/integration/autoland/rev/c9df402008ca add initialization checking to SiteIdentityPopup r=sebastian https://hg.mozilla.org/integration/autoland/rev/1edd52437ecb Add SiteIdentityPopup to CustomTabsActivity r=sebastian

https://hg.mozilla.org/mozilla-central/rev/b65a8df84b11 https://hg.mozilla.org/mozilla-central/rev/b296196d1859 https://hg.mozilla.org/mozilla-central/rev/4796c44dda66 https://hg.mozilla.org/mozilla-central/rev/540020bee3a6 https://hg.mozilla.org/mozilla-central/rev/c9df402008ca https://hg.mozilla.org/mozilla-central/rev/1edd52437ecb

Verified as fixed in build (2017-04-02); Device: Huawei Honor (Android 5.1.1).