Closed Bug 1390055 Opened 2 years ago Closed 2 years ago

should highlight library and addons at toolbar if exist

Categories

(Firefox :: Tours, enhancement, P3)

enhancement

Tracking

()

RESOLVED WONTFIX
Tracking Status
firefox57 --- wontfix
firefox58 --- wontfix

People

(Reporter: gasolin, Assigned: gasolin)

References

Details

Attachments

(1 file)

Currently, UITour will highlight the library inside the menu; we should highlight library icon at tabbar if the icon exists
Whiteboard: [photon-onboarding][triage]
also addon has the same issue, but addon should be in menu by default
Priority: -- → P2
Whiteboard: [photon-onboarding][triage] → [photon-onboarding]
per discussion with UX Verdi, change it to P3 for now.
Priority: P2 → P3
Summary: [Onboarding]should highlight library at tabbar if exist → [Onboarding]should highlight library at tab bar if exist
Summary: [Onboarding]should highlight library at tab bar if exist → [Onboarding]should highlight library and addon at tab bar if exist
Assignee: nobody → gasolin
Status: NEW → ASSIGNED
Flags: qe-verify+
Summary: [Onboarding]should highlight library and addon at tab bar if exist → [Onboarding] should highlight library and addons at tab bar if exist
Summary: [Onboarding] should highlight library and addons at tab bar if exist → should highlight library and addons at toolbar if exist
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 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.
Attachment #8898114 - Flags: review?(gijskruitbosch+bugs) → review+
issue addressed, thanks
Pushed by flin@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/fc4e3d36bf0c
should highlight library and addons at toolbar if exist;r=Gijs
https://hg.mozilla.org/mozilla-central/rev/fc4e3d36bf0c
Status: ASSIGNED → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
I have verified this issue on today's nightly.
Status: RESOLVED → VERIFIED
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.
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?
Flags: needinfo?(gijskruitbosch+bugs)
ritu provided sufficient info in bug 1406012 comment 10, thanks
Flags: needinfo?(gijskruitbosch+bugs)
Blocks: 1410763
This was backed out from both 57 and 58 in bug 1410763.
Status: VERIFIED → RESOLVED
Closed: 2 years ago2 years ago
Resolution: FIXED → WONTFIX
Target Milestone: Firefox 57 → ---
remove whiteboard tag since its wontfix
Whiteboard: [photon-onboarding]
You need to log in before you can comment on or make changes to this bug.