1390055 - should highlight library and addons at toolbar if exist

Status: RESOLVED WONTFIX

(Firefox :: Tours, enhancement, P3)

Description

[[e-mail]]

Currently, UITour will highlight the library inside the menu; we should highlight library icon at tabbar if the icon exists

Comment 1

[[e-mail]]

also addon has the same issue, but addon should be in menu by default

Comment 2

[Francis Lee [:frlee]]

per discussion with UX Verdi, change it to P3 for now.

Comment 3

[[e-mail]]

Attachment: Bug 1390055 - should highlight library and addons at toolbar if exist;

Review commit: https://reviewboard.mozilla.org/r/169450/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/169450/

Comment 4

[[e-mail]]

Comment on attachment 8898114 [details]
Bug 1390055 - should highlight library and addons at toolbar if exist;

https://reviewboard.mozilla.org/r/169450/#review174764

::: browser/base/content/browser.xul:925
(Diff revision 1)
>  
>          <toolbarbutton id="library-button" class="toolbarbutton-1 chromeclass-toolbar-additional"
>                         removable="true"
>                         oncommand="PanelUI.showSubView('appMenu-libraryView', this, null, event);"
>                         closemenu="none"
> +                       cui-areatype="toolbar"

fischer found the highlight is incorrect (a bit larger and fill the full square), the reason is the library icon lack of `cui-areatype` attribute, which is included in every customizable icons in toolbar. After add the attribute, the library icon highlight normally as other toolbar icons.

Comment 5

[:Gijs (he/him)]

Comment on attachment 8898114 [details]
Bug 1390055 - should highlight library and addons at toolbar if exist;

https://reviewboard.mozilla.org/r/169450/#review174796

r=me with the below addressed.

::: browser/components/uitour/test/browser_UITour5.js:23
(Diff revision 1)
> +    is("library-button", target.node.id, "Should highlight the right target");
> +  });
> +});
> +
> +add_UITour_task(async function test_highlight_addons_icon_in_toolbar() {
> +  CustomizableUI.addWidgetToArea("add-ons-button", CustomizableUI.AREA_NAVBAR, 0);

You should remove this button again at the end of this function.

::: browser/components/uitour/test/browser_UITour5.js:39
(Diff revision 1)
> +    is("add-ons-button", target.node.id, "Should highlight the right target");
> +  });
> +});
> +
>  add_UITour_task(async function test_highlight_library_and_show_library_subview() {
> +  CustomizableUI.removeWidgetFromArea("library-button", CustomizableUI.AREA_NAVBAR, 0);

Note that `removeWidgetFromArea` only takes 1 argument, so please remove the 2nd and 3rd one.

Comment 6

[[e-mail]]

Comment on attachment 8898114 [details]
Bug 1390055 - should highlight library and addons at toolbar if exist;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/169450/diff/1-2/

Comment 7

[[e-mail]]

issue addressed, thanks

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/fc4e3d36bf0c
should highlight library and addons at toolbar if exist;r=Gijs

Comment 9

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

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

Comment 10

[Justin [:JW_SoftvisionQA]]

I have verified this issue on today's nightly.

Comment 11

[Justin [:JW_SoftvisionQA]]

I have verified that the highlight library and addons at toolbar if exist on Win 7 x32, Win 10 x64, Mac 10.12, and Ubuntu 16.04 x32 with Firefox 58.

Comment 12

[[e-mail]]

gijs, as bug 1406012 comment 8, UX request revert this bug changes on v57 to always focus on appMenu>Library.

Do you know the process to make it happen?

Comment 13

[[e-mail]]

ritu provided sufficient info in bug 1406012 comment 10, thanks

Comment 14

[Ryan VanderMeulen [:RyanVM]]

This was backed out from both 57 and 58 in bug 1410763.

Comment 15

[Francis Lee [:frlee]]

remove whiteboard tag since its wontfix