Torbutton: Unexpected error on new identity: TypeError: b.webProgress is undefined
Anybody yet observes this alert? TBB-3.6 Everything worked fine before, and then suddenly alert message box. Last automatically updated add-on was Noscript 2.6.8.23
Debugging showing that exception generated by
browser.addTab("about:blank");from torbutton_close_on_toggle()
Can't reliably reproduce this bug, everything works and then for the same sequence of actions this fail appears.
Activity
Alert happens with freshly installed TBB-3.6 with disabled add-on updates. Didn't happen for week before and now it alerts and alerts for the same code. Suspicious.
If nobody else observes that alert then need to close bug with "works for me" resolution. "b.webProgress is undefined" appears in mozilla's bugzilla and every ticket was closed with that resolution, some race condition that unrepairable, broken by design.
Hey, cypherpunk: good news, I experienced the same problem. And even better, there is some weird stuff in the error console
[18:26:13.985] [05-10 18:26:13] Torbutton WARN: DocShell is null for unparsable URL: TypeError: this.docShell is null [18:26:14.068] [05-10 18:26:14] Torbutton WARN: Unexpected error on new identity: TypeError: b.webProgress is undefined [18:26:14.113] [05-10 18:26:14] Torbutton WARN: DocShell is null for unparsable URL: TypeError: this.docShell is null [18:26:14.113] [05-10 18:26:14] Torbutton WARN: DocShell is null for unknown URL [18:26:14.181] [05-10 18:26:14] Torbutton WARN: Unexpected error on new identity: TypeError: b.webProgress is undefined [18:26:14.397] this.docShell is null @ chrome://global/content/bindings/browser.xml:323And, yes, leave the ticket open.
If this bug still reproducible for you, can you test this:
--- popup.xul +++ popup.xul.modified @@ -14,6 +14,7 @@ <menuitem id="torbutton-new-identity" label="&torbutton.context_menu.new_identity;" accesskey="&torbutton.context_menu.new_identity_key;" + disabled="true" insertafter="context-stop" oncommand="torbutton_new_identity()"/> <menuitem id="torbutton-cookie-protector" --- torbutton.js +++ torbutton.js.modified @@ -1430,6 +1430,7 @@ // Bug 1506 P4: Needed for New Identity. function torbutton_new_identity() { + document.getElementById("torbutton-new-identity").disabled = true; try { torbutton_do_new_identity(); } catch(e) { @@ -2105,6 +2106,8 @@ if (!m_tb_control_pass || !m_tb_control_port) document.getElementById("torbutton-new-identity").disabled = true; + else + document.getElementById("torbutton-new-identity").disabled = false; if (!m_tb_tbb && m_tb_prefs.getBoolPref("extensions.torbutton.prompt_torbrowser")) { torbutton_inform_about_tbb();Can it to prevent bug?
Trac:
Username: cypherpunks_backupOr even simpler:
--- torbutton.js +++ torbutton.js.modified @@ -1430,6 +1430,7 @@ // Bug 1506 P4: Needed for New Identity. function torbutton_new_identity() { + document.getElementById("torbutton-new-identity").disabled = true; try { torbutton_do_new_identity(); } catch(e) {This should be the same. But why torbutton_new_identity called twice at all? without answer even this kludge can't guarantee such protection actually works for every case.
-
Replying to cypherpunks:
But why torbutton_new_identity called twice at all? without answer even this kludge can't guarantee such protection actually works for every case.
Not sure what you mean, but clicking on the menuitem n times is executing the function, which is bound to it, n times (approximately; it depends on how fast you and probably your computer are). Anyway, see bug_11783 in my torbutton repo for a fix.
Trac:
Keywords: N/A deleted, MikePerry201405 added
Status: reopened to needs_review but clicking on the menuitem n times is executing the function, which is bound to it, n times (approximately; it depends on how fast you and probably your computer are) You can't to click twice on the same menu item, oncommand should be called only once or menu should never be destroyed. For menu popups only 1 click is allowed, all next is state of art.
document.getElementById("torbutton-new-identity").disabled = false;Why to re-enable it on exception? What purpose? Those element doesn't exist after oncommand finished. You can't to keep menu on exception too. Element recreated on every new menu popup with default state.
-
Replying to cypherpunks:
but clicking on the menuitem n times is executing the function, which is bound to it, n times (approximately; it depends on how fast you and probably your computer are) You can't to click twice on the same menu item, oncommand should be called only once or menu should never be destroyed. For menu popups only 1 click is allowed, all next is state of art.
{{{ document.getElementById("torbutton-new-identity").disabled = false; }}} Why to re-enable it on exception? What purpose? Those element doesn't exist after oncommand finished. You can't to keep menu on exception too. Element recreated on every new menu popup with default state.
Ah, right. That's one episode in the "Think Once, Commit Twice" saga... bug_11783_v2 in my repo is the new hotness.
-
Replying to cypherpunks:
but clicking on the menuitem n times is executing the function, which is bound to it, n times (approximately; it depends on how fast you and probably your computer are) You can't to click twice on the same menu item, oncommand should be called only once or menu should never be destroyed. For menu popups only 1 click is allowed, all next is state of art.
You might be right here. However, this is not the whole story. I see in fact the second execution in my logs although only after we called OpenBrowserWindow(). My guess is that the second execution happens somehow in this new window context which is not properly initialized yet. Anyway, I don't have the time to look deeper at it right now to understand what is exactly going on under the hood.
My guess is that the second execution happens somehow in this new window context which is not properly initialized yet.
I think second execution happens for the same menu popup as for first click. Browser keeps menu while context was destroyed or should be destroyed, race condition is about order of destroy for menu popup vs tabs. Or no races even, browser destroys context of window first while menu exist till the end. Bug is about speedness of user vs. speed of code execution (possible depends jit?).
I think we want to take the version that re-enables the new identity button if there was an exception. That menu will not be redrawn each time it is opened, as it is sourced from the torbutton.xul overlay, which is bound to browser.xul. It will only be re-created if the browser window is successfully closed and a new one is opened.
This makes me think that gk/bug_11783 is the right branch to merge here, so that exceptions don't completely disable New Identity.
I think we want to take the version that re-enables the new identity button if there was an exception. That menu will not be redrawn each time it is opened, as it is sourced from the torbutton.xul overlay, which is bound to browser.xul. It will only be re-created if the browser window is successfully closed and a new one is opened.
This theory easy to test, no need to guess.
-
Replying to cypherpunks:
it need to re-enable menu item if caught exception. But as shown this bug, every next click in the same context could leads to crash/exit.
Hrmm.. I wonder why the crash/exit is happening.. The undefined error should not cause that.. Leaving New Identity greyed seems to just hide that issue?
Should we file a different ticket for the crash? Should I revert this merge?
Trac:
Keywords: MikePerry201405R deleted, MikePerry201406R added -
Replying to mikeperry:
Ok, so on IRC gk and I agreed to file a different ticket for the unexpected exit issue.
On a second thought comment 39 is wrong and we don't need a new ticket. The "click-and-the browser-crashes"-issue existed only due to the bug we fixed here. So, re-enabling the "New Identity"-option is fine and won't lead to a browser shutdown anymore. Everything is good.