908162 - Removes Button module's dependency from Sidebar module

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: General, defect)

Description

[Matteo Ferretti [:zer0] [:matteo]]

As agreed, Sidebar and Toolbar components won't have mandatory button as requirement.

This bug is intended for remove the Button from Sidebar and update the tests – that should also "fix" bug 906690.

As consequences, the Sidebar module will became non-Australis compatible, and will be possible to use right now as "low level" module (the devs needs to require explicitly `sdk/ui/sidebar`), where it will be available as "high level" module ('sdk/ui') together with the rest of UI components, once Australis will land.

Comment 1

[Matteo Ferretti [:zer0] [:matteo]]

Attachment: Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1216

Pointer to Github pull-request

Comment 2

[Erik Vold [:erikvold] (please needinfo? me)]

Comment on attachment 793977 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1216

Mainly some tests were removed and should be rewritten to use a menuitem instead of a button instead afaict.  These tests should have existed already I admit, but it will be harder to add them again when these tests are removed and we have to dig for them, and possibly deal with bit rot.

Comment 3

[Matteo Ferretti [:zer0] [:matteo]]

Erik, this is blocking fx-team for Australis since a while. If the only thing here are the tests that were already missed, I suggest to land this as soon as possible and then open a new bug to add those tests. IMVHO it's not also the scope of this bug having those tests in the same PR – also, I would like avoid introducing new things in the same pull, in order to have a better granularity about what pull introduced issues on Australis, is something going wrong again only on UX build on some machines.

Comment 4

[Matteo Ferretti [:zer0] [:matteo]]

You know what: because we're in the work week, let's do those tests together today, in a pair coding / review, to speed up.

Comment 5

[Erik Vold [:erikvold] (please needinfo? me)]

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #3)
> Erik, this is blocking fx-team for Australis since a while. If the only
> thing here are the tests that were already missed, I suggest to land this as
> soon as possible and then open a new bug to add those tests. IMVHO it's not
> also the scope of this bug having those tests in the same PR – also, I would
> like avoid introducing new things in the same pull, in order to have a
> better granularity about what pull introduced issues on Australis, is
> something going wrong again only on UX build on some machines.

I'm not asking for something new afaict.  The tests I commented on are two parts.  First, using the button to open a sidebar, and second using a alternative method to close the sidebar.  The first part should be removed as part of the scope of this bug, and since we are not testing the latter piece in any other tests they should not be removed.  Otherwise we have no test for using the X button to close a sidebar for example, or no test for closing a sidebar in a second window as another example.

(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #4)
> You know what: because we're in the work week, let's do those tests together
> today, in a pair coding / review, to speed up.

Sure

Comment 6

[[github robot]]

Commits pushed to master at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/f732c9e1d1d0b26e1357a7f4fa0be78aff7ea9f3
Bug 908162 - Removes Button module's dependency from Sidebar module

 - removed `sdk/ui/button` dependencies and `sdk/ui/state` dependencies
 - removed `icon` property
 - removed `CustomizableUI` dependencies
 - thrown a new Error if the ID is already used in create method of `ui/sidebar/view`
 - updated tests
 - replaced metadata: sidebar is not compatible only with australis now
 - replaced deprecated for each loop with `for..of`
 - disabled `testSidebarIsOpenInNewPrivateWindow` in `private-browsing-supported`

https://github.com/mozilla/addon-sdk/commit/a38c9e17e471ecf602782b22b8222bc9f2e813a3
Merge pull request #1216 from ZER0/sidebar/908162

fix Bug 908162 - Removes Button module's dependency from Sidebar module r=@erikvold