Closed
Bug 878905
Opened 11 years ago
Closed 11 years ago
improve camera button menuitem labels
Categories
(Firefox :: General, defect)
Tracking
()
RESOLVED
FIXED
Firefox 27
People
(Reporter: mixedpuppy, Assigned: mixedpuppy)
Details
Attachments
(1 file, 1 obsolete file)
(deleted),
patch
|
dao
:
review+
|
Details | Diff | Splinter Review |
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.
Assignee | ||
Comment 1•11 years ago
|
||
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.
Attachment #757632 -
Flags: feedback?(dolske)
Assignee | ||
Comment 2•11 years ago
|
||
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?
Flags: needinfo?(jboriss)
Updated•11 years ago
|
OS: Mac OS X → All
Hardware: x86 → All
Comment 3•11 years ago
|
||
(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."
Flags: needinfo?(jboriss)
Assignee | ||
Comment 4•11 years ago
|
||
(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.
Assignee | ||
Updated•11 years ago
|
Attachment #757632 -
Flags: feedback?(dolske) → review?(dao)
Assignee | ||
Comment 5•11 years ago
|
||
@dao ping
Comment 6•11 years ago
|
||
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.
Attachment #757632 -
Flags: review?(dao) → review-
Assignee | ||
Comment 7•11 years ago
|
||
updated with feedback.
Assignee: nobody → mixedpuppy
Attachment #757632 -
Attachment is obsolete: true
Attachment #811347 -
Flags: review?(dao)
Updated•11 years ago
|
Attachment #811347 -
Flags: review?(dao) → review+
Assignee | ||
Comment 8•11 years ago
|
||
Assignee | ||
Comment 9•11 years ago
|
||
Comment 10•11 years ago
|
||
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 27
You need to log in
before you can comment on or make changes to this bug.
Description
•