Build Identifier: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:2.0b8pre) Gecko/20101016 Firefox/4.0b8pre ID:20101016041245 Tooltip for Icon and Searchbox should be provided in Panorama Reproducible: Always Steps to Reproduce: 1. Start Minefield with new profile 2. Enter Panorama Ctri+E 3. Mouse hiver icon Actual Results: No tooltip indicated.

We're punting all our accessibility bugs to milestone "future". Though these are first on our list of stretch-goals.

Involves strings so unfortunately it's too late for fx4. Punting.

Comment on attachment 555346 [details] [diff] [review] v1 aHTMLTooltip is there for content documents and needs a rewrite for e10s, please don't use it.

Comment on attachment 555346 [details] [diff] [review] v1 Review of attachment 555346 [details] [diff] [review]: ----------------------------------------------------------------- Thanks for tackling this! ::: browser/base/content/browser-tabview.js @@ +189,5 @@ > // ___ create the frame > this._iframe = document.createElement("iframe"); > this._iframe.id = "tab-view"; > this._iframe.setAttribute("transparent", "true"); > + this._iframe.setAttribute("tooltip", "aHTMLTooltip"); Please address Dão's comment. We don't need to be e10s safe because the Panorama iframe won't run in a separate process but we'd need our own implementation of "aHTMLTooltip". Maybe we don't need the "title" attributes either with our own solution. ::: browser/base/content/tabview/groupitems.js @@ +198,5 @@ > e.stopPropagation(); > }) > .keypress(handleKeyPress) > + .keyup(handleKeyUp) > + .attr("title", tabviewString("groupItem.defaultName")); I agree, that the titleShield should have a tooltip, but should the input field have one, too? ::: browser/base/content/tabview/tabitems.js @@ +913,3 @@ > tabItem.url = tabUrl; > tabItem.save(); > + tabItem.$canvas.attr("title", tabUrl); Could we maybe set a "title" for the whole tabItem and combine it like "$label - $url"? IMHO it's not that useful to differentiate between the title and the url. ::: browser/locales/en-US/chrome/browser/tabview.properties @@ +1,1 @@ > +tabview.button.searchTabs=Search tabs... That should be "Search tab groups" @@ +1,2 @@ > +tabview.button.searchTabs=Search tabs... > +tabview.button.exitTabGroups=Exit Tabs Groups IMHO this should be "Exit Panorama". https://www.mozilla.com/en-US/firefox/6.0/whatsnew/ refers to us as "Panorama". @@ +1,3 @@ > +tabview.button.searchTabs=Search tabs... > +tabview.button.exitTabGroups=Exit Tabs Groups > +tabview.groupItem.defaultName=Name this tab group... You accidentally replaced "…" (…) with "..." (three dots). @@ +5,2 @@ > tabview.groupItem.undoCloseGroup=Undo Close Group > +tabview.groupItem.restoreClosedGroup=Restore closed group This should rather be "Discard closed group". The close icon does not restore but remove the hidden group. @@ +5,3 @@ > tabview.groupItem.undoCloseGroup=Undo Close Group > +tabview.groupItem.restoreClosedGroup=Restore closed group > +tabview.tabItem.closeTab=Close tab We should re-use "tabs.closeTab" here.

> Please address Dão's comment. We don't need to be e10s safe because the > Panorama iframe won't run in a separate process but we'd need our own > implementation of "aHTMLTooltip". Maybe we don't need the "title" attributes > either with our own solution. Implemented our own fillInTooltip method. > > Could we maybe set a "title" for the whole tabItem and combine it like > "$label - $url"? IMHO it's not that useful to differentiate between the > title and the url. The whole tabItem has a two line tooltip: first line is label and second line is url. Fixed other things in the properties file

Comment on attachment 555429 [details] [diff] [review] v2 >+ document.documentElement.appendChild(tooltip); Can you find a more sensible place for this? mainPopupSet?

> Can you find a more sensible place for this? mainPopupSet? Fixed

Comment on attachment 555616 [details] [diff] [review] v3 Review of attachment 555616 [details] [diff] [review]: ----------------------------------------------------------------- Sorry, I forgot that from an accessibility point of view it could make sense to have a title attribute for the group title's input field. Can you re-include that, please? :) Anyway, looks good! I like that newline solution for the tab tooltips, too. r=me with those two little fixes. I'd like to have UX weigh in. Can you please push that patch to try and and ask the UX team for review and point to your try builds? That would be great, thanks. ::: browser/locales/en-US/chrome/browser/tabview.properties @@ +3,2 @@ > tabview.groupItem.defaultName=Name this tab group… > +tabview.groupItem.closeGroup=Close Group Please make that a lowercase "group".

(In reply to Tim Taubert [:ttaubert] from comment #11) > Sorry, I forgot that from an accessibility point of view it could make sense > to have a title attribute for the group title's input field. Can you > re-include that, please? :) I didn't remove it in this patch. > ::: browser/locales/en-US/chrome/browser/tabview.properties > @@ +3,2 @@ > > tabview.groupItem.defaultName=Name this tab group… > > +tabview.groupItem.closeGroup=Close Group > > Please make that a lowercase "group". Fixed Could UX weight in this please? You can download the builds using the below link http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/raymond@raysquare.com-4720bf706bd1/

unrotted Passed Try: https://tbpl.mozilla.org/?tree=Try&rev=d8f36b3841ff Builds available in the below address http://ftp.mozilla.org/pub/mozilla.org/mobile/try-builds/raymond@raysquare.com-d8f36b3841ff/ Alex: could you weight in please? Thanks!

Does this add a tooltip of tab title to the tab thumbnail itself?

(In reply to Dietrich Ayala (:dietrich) from comment #15) > Does this add a tooltip of tab title to the tab thumbnail itself? Nm, that's bug 587301.

(In reply to Dietrich Ayala (:dietrich) from comment #15) > Does this add a tooltip of tab title to the tab thumbnail itself? Yes, this does add a tooltip of tab title to the tab thumbnail.

Updated the patch after updating the patch. limi: please see the video which shows how the tooltips look like https://bugzilla.mozilla.org/attachment.cgi?id=569914

Comment on attachment 569915 [details] [diff] [review] v6 Passed Try! https://tbpl.mozilla.org/?tree=Try&rev=0cf0e38a1cf9

Two quick things re tooltip strings: 1. I see in Raymond's video in Comment 18 that the tooltip for exiting says "Exit Panorama." As far as I know we don't use the name Panorama in product any longer, so this should just be "Exit tab groups." 2. The text field tooltip for naming a tab group has "..." after it. We don't do this for other Firefox text fields, so let's shorten the strong to just "Name this tab group"

Video looks great! Can't wait to see this slipped into Nightly!

Comment on attachment 569915 [details] [diff] [review] v6 Sorry about not seeing this earlier, ui-r+ with the changes Boriss pointed out. Minor nit: Don't include the punctuation mark in any of the tooltips, tooltips never have an ending period unless there are multiple sentences (in which case we're probably doing it wrong :) (Using this bug to show Zhenshuo and Yuan how Bugzilla reviews work, didn't mean to steal from you. :)

(In reply to Jennifer Morrow [:Boriss] (Firefox UX) from comment #21) > Two quick things re tooltip strings: > > 1. I see in Raymond's video in Comment 18 that the tooltip for exiting says > "Exit Panorama." As far as I know we don't use the name Panorama in product > any longer, so this should just be "Exit tab groups." > Updated > 2. The text field tooltip for naming a tab group has "..." after it. We > don't do this for other Firefox text fields, so let's shorten the strong to > just "Name this tab group" Fixed Pushed to try and waiting for results https://tbpl.mozilla.org/?tree=Try&rev=dc3660b678e8

(In reply to Raymond Lee [:raymondlee] from comment #24) > Pushed to try and waiting for results > https://tbpl.mozilla.org/?tree=Try&rev=dc3660b678e8 Passed Try