437955 - Page Identity

Status: VERIFIED FIXED

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

Description

[Christian Sejersen]

Comment 1

[Johnathan Nightingale [:johnath]]

Attachment: Add identity panel, attached to favicon in address bar

I think the easiest way to do this is twofold:

 1) Port over the existing code from Firefox that handles the onSecurityChange events in order to track current identity information.
 2) Plug it in to the fennec UI in a design-appropriate way.

I think the best way forward is to do these things independently. I can port the Firefox code over and hang it off the favicon temporarily (non-discoverable, but allows the logic to land independent of the presentation) and then, once we settle on a design for basic navigation and security information, re-style the identity UI to fit.

To that end - here's a patch that adds an FF3-style panel, activated by favicon tap, with identity information.

It requires two new image files, which I'll attach separately.

Comment 2

[Johnathan Nightingale [:johnath]]

Attachment: Vista-style identity ("Larry") icons

Comment 3

[Johnathan Nightingale [:johnath]]

Attachment: 24x24 Padlock image (used in identity panel)

Comment 4

[Johnathan Nightingale [:johnath]]

Attachment: Minor updates

Minor changes to remove windows-specific cruft and commented out code.  Obviously this will change once we have an official primary chrome.

Comment 5

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

Comment on attachment 325995 [details] [diff] [review]
Minor updates

Just some drive by comments

>diff -r 550f014628ba chrome/content/browser.js

>+    var location = getBrowser().contentWindow.location;

Everytime I see this I wonder if getBrowser().currentURI (nsIURI) would be better than dipping into the content to get 'location'. Anybody know?

>+/**
>+ * Utility class to handle manipulations of the identity indicators in the UI
>+ */
>+function IdentityHandler() {

Since the UI lives in toolbar.xul, I wonder if the JS should live in toolbar.js? Of course the CSS is in browser.css _and_ the UI will change anyway. Just something we'll need to reorganize later.

Comment 6

[:Gavin Sharp [email: [e-mail]]]

(In reply to comment #5)
> Everytime I see this I wonder if getBrowser().currentURI (nsIURI) would be
> better than dipping into the content to get 'location'. Anybody know?

The location object does basically the same thing as browser.xml's currentURI getter, with some extra cleanup that we would need to do here anyways:

http://bonsai.mozilla.org/cvsblame.cgi?file=mozilla/dom/src/base/nsLocation.cpp&rev=1.150&mark=327-340#306

It could be that there's some wrapper overhead from going through the content window, but it's hard to tell how that compares to the overhead of calling that cleanup code through xpconnect.

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Comment on attachment 325995 [details] [diff] [review]
Minor updates

This looks good (would have been useful to get a list of changes needed to adapt it to Fennec, though, since that's what really needs reviewing).

I wonder whether we can put most of this code (the code that doesn't deal directly with UI, anyways?) into a JS module and share it with Firefox directly rather than doing all this copying. Also it'd be nice if we could share as much of the strings/CSS through build-time source file copying tricks rather than having to maintain a fork. We can look into that in followups.

We can take this for M4, feel free to push it to mobile-browser.

Comment 8

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

(In reply to comment #7)

> I wonder whether we can put most of this code (the code that doesn't deal
> directly with UI, anyways?) into a JS module and share it with Firefox directly
> rather than doing all this copying. Also it'd be nice if we could share as much
> of the strings/CSS through build-time source file copying tricks rather than
> having to maintain a fork. We can look into that in followups.

Excellent idea

Comment 9

[Johnathan Nightingale [:johnath]]

Committed.

changeset:   33:69cd6684e101
changeset:   32:3032efb61657

Before verifying this as fixed, I will file two follow-ups:

 - Look for ways to make this code modular between firefox and fennec (less easy than it seems, since a subset of code, subset of strings, and subset of styling is used in fennec, but probably still quite doable for most things).

 - Hook this up to some form of discoverable UI - either a button on the bow or a descender from the title, or something other than a favicon which happens to be clickable.

Comment 10

[Joel Maher ( :jmaher ) (UTC -8)]

verified with beta3