Closed Bug 787395 Opened 12 years ago Closed 11 years ago

Add Sidebars components type

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: zer0, Assigned: evold)

References

(Depends on 1 open bug, )

Details

Attachments

(4 files)

As the UX mockups shows:

http://people.mozilla.com/~shorlander/files/addons-in-toolbar-i01/addons-in-toolbar.html

Some widgets needs to display vertical content that doesn't overlap with the content of the current tab (e.g. Twitter's stream, devtools, etc).
Blocks: 695913
Might also be nice to have an option to have the bar along the bottom of the window instead of the side, for content that's wider than it is tall.
Priority: -- → P1
"Arbitrary chrome priv'd html, stuck onto anything in a box" is still my holy grail on this stuff :)  At least allow style and placement overrides!

(Use case:  deep ui experiments, like icon based right click menus)
Assignee: nobody → evold
OS: Mac OS X → All
Hardware: x86 → All
(In reply to Gregg Lind (User Research - Test Pilot) from comment #2)
> "Arbitrary chrome priv'd html, stuck onto anything in a box" is still my
> holy grail on this stuff :)  At least allow style and placement overrides!

That sounds good to me :)
Summary: Add Sidebars widgets type → Add Sidebars components type
Depends on: 838797
Comment on attachment 750243 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/997

I'm reusing the built-in sidebar here, we could implement our own though.
Attachment #750243 - Flags: feedback?(zer0)
Attachment #750243 - Flags: feedback?(dtownsend+bugmail)
(In reply to Erik Vold [:erikvold] [:ztatic] from comment #7)
> Comment on attachment 750243 [details]
> Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/997
> 
> I'm reusing the built-in sidebar here, we could implement our own though.

Here is an example:

require('sdk/ui/sidebar').SideBar({
  id: 'test-example-sidebar-thing',
  label: 'TEST',
  url: require('self').data.url('index.html')
}).show();
Comment on attachment 750243 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/997

Along the right lines but I think we've had discussions about whether or not to share open/closed state across windows. I think the result was that we don't share state but a newly opened window will take the state of the last window, ZER0 can confirm that. This code seems to match that except it doesn't look like it supports newly opened windows at all. We'll probably want to add to the API to be able to test whether the sidebar is open or closed for a given window.
Attachment #750243 - Flags: feedback?(dtownsend+bugmail) → feedback+
Depends on: 877867
No longer depends on: 838797
I've upgraded the dog food, it now tastes better.

There are still some weird bugs tho, like causing crashes if I use window.toggleSidebar() to close the sidebar, and the show event isn't emitted for a test when all of the tests are run together, but when I run them separately they work..

I'll have to look in to it all more tomorrow morning.

The branch is darn close I think, it took many hacks..
Depends on: 881011
No longer depends on: 877867
Depends on: 881542, 881008
Just to be clear I don't think the 3 current bugs that this bug depends on are blockers, I have hacked around them so far and the hacks seem to work.
Comment on attachment 761186 [details]
Pointer to Github pull request: https://github.com/mozilla/addon-sdk/pull/1027

Alright this is at least 80% of the way to done, I will add some tests, but the sidebar component is mostly done and ready for a review I think.
Attachment #761186 - Flags: review?(zer0)
Attachment #761186 - Flags: feedback?(dtownsend+bugmail)
Attachment #750243 - Flags: feedback?(zer0)
Alright, I've written all of the tests that I wanted to do, so I just need to merge this branch with the button branch now.
Attachment #761186 - Flags: feedback?(dtownsend+bugmail) → feedback+
Depends on: 884136
Depends on: 885054
Note bug 885638 another toggleSidebar bug
Ok, here is the crash causing code example, try this in Scratchpad:

let sidebar = window.document.getElementById('sidebar');
sidebar.addEventListener('load', function() {
  let panel = sidebar.contentDocument.getElementById('web-panels-browser');
  panel.addEventListener('DOMWindowCreated', function() {
    window.toggleSidebar();
  })
}, true)
window.openWebPanel('test', 'https://mozilla.org');

And BOOM! Fx crashes.
Not sure if it's related, but I had a similar issue in the past – maybe you remember the "Crash Me" add-on I made during a weekly meeting.

So the issue was that the document inside was still loading, and close the container immediately made Firefox unhappy. So I basically waited for the document to be "interactive". Following your code, it should be:

let sidebar = window.document.getElementById('sidebar');
sidebar.addEventListener('load', function() {
  let panel = sidebar.contentDocument.getElementById('web-panels-browser');
  panel.addEventListener('DOMWindowCreated', function(event) {
    let document = event.explicitOriginalTarget;
    
    document.addEventListener('readystatechange', function ready() {
      if (this.readyState === 'interactive') {
        document.removeEventListener('readystatechange', ready);
        window.toggleSidebar();
      }
    })
  })
}, true)
window.openWebPanel('test', 'https://mozilla.org');

I used this workaround until the bug was fixed, not sure if it makes sense in your context but at least it's an additional clue that can be used to fix the bug on platform side.
Depends on: 885949
No longer depends on: 881011
Depends on: 886148
Note: It appears that sidebars are not leaking, so the crash causing code in window.toggleSidebar (see bug 885949) isn't necessary to avoid leaks, at least in our use of web panel sidebars.
(In reply to Matteo Ferretti [:matteo] [:zer0] from comment #17)
> Not sure if it's related, but I had a similar issue in the past – maybe you
> remember the "Crash Me" add-on I made during a weekly meeting.
> 
> So the issue was that the document inside was still loading, and close the
> container immediately made Firefox unhappy. So I basically waited for the
> document to be "interactive". Following your code, it should be:
> 
> let sidebar = window.document.getElementById('sidebar');
> sidebar.addEventListener('load', function() {
>   let panel = sidebar.contentDocument.getElementById('web-panels-browser');
>   panel.addEventListener('DOMWindowCreated', function(event) {
>     let document = event.explicitOriginalTarget;
>     
>     document.addEventListener('readystatechange', function ready() {
>       if (this.readyState === 'interactive') {
>         document.removeEventListener('readystatechange', ready);
>         window.toggleSidebar();
>       }
>     })
>   })
> }, true)
> window.openWebPanel('test', 'https://mozilla.org');
> 
> I used this workaround until the bug was fixed, not sure if it makes sense
> in your context but at least it's an additional clue that can be used to fix
> the bug on platform side.

I think this could introduce close lag and race conditions issues that are avoidable.
Depends on: 887666
Depends on: 887908
Commit pushed to australis at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/86f3d5419ac8c31551de345a1f9d4072f4c4ce0d
Bug 787395 Adding a Sidebar module r=@ZER0

Conflicts:
	test/test-url.js
Commit pushed to australis at https://github.com/mozilla/addon-sdk

https://github.com/mozilla/addon-sdk/commit/613c6496a035b1c1846c8db36936f32673f879bd
Bug 787395 Adding a Sidebar module r=@ZER0

Conflicts:
	test/test-url.js
Attachment #761186 - Flags: review?(zer0)
Attachment #766805 - Flags: review+
Is this fixed now?
I'm going through the list of open bugs that github robot has commented in. Is this bug fixed, Matteo?
Flags: needinfo?(zer0)
I would say yes.
Flags: needinfo?(zer0)
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Wow, I didn't know this API. My sample restartless extension https://github.com/kyoshino/simple-sidebar is no longer needed!

Updated the doc: https://developer.mozilla.org/en-US/docs/Creating_a_Firefox_sidebar
You need to log in before you can comment on or make changes to this bug.