878905 - improve camera button menuitem labels

Status: RESOLVED FIXED

(Firefox :: General, defect)

Description

[Shane Caraveo (:mixedpuppy)]

Started conversation in bug 871931, look there for previous details.

The issue is that urls in the button are not very good indicators to the user.  The document title would potentially be better.  Another option is to have both document title and url.  The favicon is probably a good idea as well.

Comment 1

[Shane Caraveo (:mixedpuppy)]

Attachment: use favicon and document title

Here's a quick patch that adds favicon and uses doc title for the label.  I think that the icon is quite useful for quick/easy identification if the user has streams open from more than one site.  The url is still in the tooltip.

Comment 2

[Shane Caraveo (:mixedpuppy)]

Boriss, the original mockups for the camera button in bug 799417 used the page url as the label of the menuitem.  The patch here would change that to favicon + document title.  For something like Talkilla, there are likely to be multiple entries with a very similar url, making the current menu (using urls) relatively ineffective.  What do you think of this change?

Comment 3

[Jennifer Morrow [:Boriss] (UX)]

(In reply to Shane Caraveo (:mixedpuppy) from comment #2)
> Boriss, the original mockups for the camera button in bug 799417 used the
> page url as the label of the menuitem.  The patch here would change that to
> favicon + document title.  For something like Talkilla, there are likely to
> be multiple entries with a very similar url, making the current menu (using
> urls) relatively ineffective.  What do you think of this change?

Makes sense not to use URLs.  As Dolske notes in bug 871931#c6, users have already granted permission to the page on the page itself, so if a page is spoofing a page title it's hardly the most important security concern at that point.  Also, granting access to multiple URLs on the same domain would make this a nightmare.  

For something like Talkilla or a video chat in SocialAPI, what would "contentDocument.title" entail?  New strings may be needed for some of our projects that ask for video permission.  The title bar of a chat in SocialAPI currently just has the name of the person being contacted, where really we'd want something more like "Bill Hicks - chat" or "Bill Hicks - video chat."

Comment 4

[Shane Caraveo (:mixedpuppy)]

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #3)
> (In reply to Shane Caraveo (:mixedpuppy) from comment #2)

> For something like Talkilla or a video chat in SocialAPI, what would
> "contentDocument.title" entail?  New strings may be needed for some of our
> projects that ask for video permission.  The title bar of a chat in
> SocialAPI currently just has the name of the person being contacted, where
> really we'd want something more like "Bill Hicks - chat" or "Bill Hicks -
> video chat."

The document title is defined by the web page loaded into the window.  We could append something to that, but I'm figuring the favicon and title is enough to identify what it is.  I'd prefer to stick to document title for now, and modify further later if there is a need.

Comment 5

[Shane Caraveo (:mixedpuppy)]

@dao ping

Comment 6

[Dão Gottwald [:dao]]

Comment on attachment 757632 [details] [diff] [review]
use favicon and document title

>-      menuitem.setAttribute("label", streamData.uri);
>+      menuitem.setAttribute("class", "menuitem-iconic");
>+      menuitem.setAttribute("label", streamData.browser.contentDocument.title || pageURI.host);

Accessing browser.contentDocument.title is bad for e10s. You should use browser.contentTitle, which is managed by the remote-browser binding.

I'm not sure we should reduce the label to the host instead of the full URI in case there's no contentTitle...

>+      PlacesUtils.favicons.getFaviconURLForPage(pageURI, function (aURI) {
>+        if (aURI) {
>+          let iconURL = PlacesUtils.favicons.getFaviconLinkForIcon(aURI).spec;
>+          menuitem.style.listStyleImage = "url(" + iconURL + ")";

Setting menuitem.image should work here.

Comment 7

[Shane Caraveo (:mixedpuppy)]

Attachment: use favicon and document title

updated with feedback.

Comment 8

[Shane Caraveo (:mixedpuppy)]

https://tbpl.mozilla.org/?tree=Try&rev=11a3d5101a08

Comment 9

[Shane Caraveo (:mixedpuppy)]

fresh try from last week:
https://tbpl.mozilla.org/?tree=Try&rev=06e7ccc68793


https://hg.mozilla.org/integration/fx-team/rev/a4c2df11b142

Comment 10

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/a4c2df11b142