Trying to close the sidebar when it is already closed causes an error

NEW
Unassigned

Status

()

5 years ago
5 years ago

People

(Reporter: evold, Unassigned)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Try the following in Scratchpad when the sidebar is closed:

    window.toggleSidebar()
    /*
    Exception: sidebarBroadcaster is null
    toggleSidebar@chrome://browser/content/browser.js:12151
    @Scratchpad/1:1
    */

imo there should be no error.
Blocks: 787395
fwiw I copied the code that hides the sidebar from toggleSidebar in to jetpack, with the only change of commenting out `sidebar.docShell.createAboutBlankContentViewer(null)` which is working without causing Firefox to crash, so I'm not sure why this is causing crashes.
Component: General → Toolbars and Customization
The code that causes the error was introduced in bug 728426

http://hg.mozilla.org/mozilla-central/rev/a42fbbc49fbb

Maybe Tim can help?
Flags: needinfo?(ttaubert)
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.
It's crashing in nsGlobalWindow::DispatchDOMWindowCreated():

http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.cpp#2619

toggleSidebar() calls createAboutBlankContentViewer() and probably destroys mDoc. Looks like we should just bail out if (!mDoc).
Flags: needinfo?(ttaubert)
Referring to comment #0, toggleSidebar() is throwing because no 'commandID' argument is given and no 'sidebarcommand' attribute is defined on '#sidebar-box'. We could check if 'sidebarBroadcaster' is valid but the result would be the same.
(In reply to Tim Taubert [:ttaubert] from comment #5)
> Referring to comment #0, toggleSidebar() is throwing because no 'commandID'
> argument is given and no 'sidebarcommand' attribute is defined on
> '#sidebar-box'. We could check if 'sidebarBroadcaster' is valid but the
> result would be the same.

Ya this bug is for two different issues atm, I'll make a new bug.

I don't think calling window.toggleSidebar() when the sidebar is closed should cause an error.  If `sidebarBroadcaster` is null then `window.toggleSidebar()` should return, and not error imo.
(In reply to Tim Taubert [:ttaubert] from comment #4)
> It's crashing in nsGlobalWindow::DispatchDOMWindowCreated():
> 
> http://mxr.mozilla.org/mozilla-central/source/dom/base/nsGlobalWindow.
> cpp#2619
> 
> toggleSidebar() calls createAboutBlankContentViewer() and probably destroys
> mDoc. Looks like we should just bail out if (!mDoc).

I made bug 885948 for this.
No longer blocks: 787395
Blocks: 885949
You need to log in before you can comment on or make changes to this bug.