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

Status: RESOLVED WONTFIX

(Firefox :: Toolbars and Customization, defect)

Description

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

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.

Comment 1

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

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.

Comment 2

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

The code that causes the error was introduced in bug 728426

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

Maybe Tim can help?

Comment 3

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

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.

Comment 4

[Tim Taubert [:ttaubert] (inactive)]

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).

Comment 5

[Tim Taubert [:ttaubert] (inactive)]

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.

Comment 6

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

(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.

Comment 7

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

(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.

Comment 8

[:Gijs (he/him)]

Given there are no more XPCOM add-ons, I think we can close this.