User Agent: Mozilla/5.0 (X11; Linux i686; rv:16.0) Gecko/16.0 Firefox/16.0 Build ID: 20120712030541 Steps to reproduce: I have restarted Daily after the latest upgrade (about 2 - 3 hours ago) and I chose to compose a new e-mail message. I pasted a wrong link into the message so decided that I will CTRL + Z (undo) it. Actual results: It turns out that the key kombo was not working. I checked CTRL + A, CTRL + C and CTRL + V combos and this are working fine. Edit dropdown menu has Undo entry grayed out. Expected results: I really love this part of the bug reporting but... I will humor You ;). The CTRL + Z control key press should undo the wrong link.
Regression window: Good: http://hg.mozilla.org/mozilla-central/rev/4b1249ae1906 http://hg.mozilla.org/comm-central/rev/a9ea38d7a17c Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Thunderbird/16.0a1 ID:20120706030536 Bad: http://hg.mozilla.org/mozilla-central/rev/6d7fae9764b3 http://hg.mozilla.org/comm-central/rev/7b0299689777 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Thunderbird/16.0a1 ID:20120707030544 Pushlog: http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=4b1249ae1906&tochange=6d7fae9764b3 http://hg.mozilla.org/comm-central/pushloghtml?fromchange=a9ea38d7a17c&tochange=7b0299689777 I guess regression of Bug 765595
Thanks for the report. I'm not able to reproduce in Firefox. Can you reproduce in Firefox on this page? <http://www-archive.mozilla.org/editor/midasdemo/> That would help me track down and fix the problem much more easily. Thanks! (In reply to Alice0775 White from comment #1) > I guess regression of Bug 765595 Seems possible, since that messed around with transaction-related code a bunch, although I'm not sure exactly how it would have happened. Thanks for the regression range!
(In reply to :Aryeh Gregor from comment #2) > Thanks for the report. I'm not able to reproduce in Firefox. Can you > reproduce in Firefox on this page? > <http://www-archive.mozilla.org/editor/midasdemo/> That would help me track > down and fix the problem much more easily. Thanks! This problem only happens with Compose window of Thunderbird and SeaMonkey. And I confirmed in Thunderbird local build, this is regression of Bug 765595.
In reply to :Aryeh Gregor from comment #2) The following Ritch Text Editor are also not working(CTRL+Z) http://www.kevinroth.com/rte/demo.htm http://developer.yahoo.com/yui/examples/resize/rte_resize.html Regression range (mozilla-inbound) Good: http://hg.mozilla.org/integration/mozilla-inbound/rev/0158f2d0b32d Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120705225257 Bad; http://hg.mozilla.org/integration/mozilla-inbound/rev/510602478d52 Mozilla/5.0 (Windows NT 6.1; WOW64; rv:16.0) Gecko/16.0 Firefox/16.0 ID:20120706005656 Pushlog: http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=0158f2d0b32d&tochange=510602478d52
Yeah, undo completely doesn't work. That's very bad. I'll look.
Minimal test-case: data:text/html,<!DOCTYPE html> <iframe></iframe> <script> onload = function() { var doc = document.body.firstChild.contentDocument; doc.body.innerHTML = "<p>Hello"; doc.designMode = "on"; doc.designMode = "off"; doc.designMode = "on"; doc.getSelection().selectAllChildren(doc.body.firstChild); doc.execCommand("bold"); document.body.textContent = doc.queryCommandEnabled("undo"); } </script> Outputs "false", should output "true". One-line fix coming up.
Created attachment 641789 [details] [diff] [review] Patch Bug 765595 part 2 basically did this to nsEditor::EnableUndo, after cleanup: NS_IMETHODIMP nsEditor::EnableUndo(bool aEnable) { if (aEnable) { if (!mTxnMgr) { - mTxnMgr = do_CreateInstance(NS_TRANSACTIONMANAGER_CONTRACTID, &result); - if (NS_FAILED(result) || !mTxnMgr) { - return NS_ERROR_NOT_AVAILABLE; - } + mTxnMgr = new nsTransactionManager(); } - mTxnMgr->SetMaxTransactionCount(-1); } else if (mTxnMgr) { // disable the transaction manager if it is enabled mTxnMgr->Clear(); mTxnMgr->SetMaxTransactionCount(0); } return NS_OK; } I removed the SetMaxTransactionCount(-1) line because I saw that the nsTransactionManager constructor did that anyway, and now I was explicitly using the constructor. But of course, this is totally wrong, because the constructor would only run if mTxnMgr was false! So the first time something called EnableUndo(true), it would create a new nsTransactionManager with its max transaction count correctly initialized to -1. But if something then called EnableUndo(false) and then EnableUndo(true), such as by toggling designMode on and off, the max transaction count would remain stuck at 0, stopping any undos or redos. Try: https://tbpl.mozilla.org/?tree=Try&rev=6e0abc054287
(In reply to :Aryeh Gregor from comment #7) > I removed the SetMaxTransactionCount(-1) line because I saw that the > nsTransactionManager constructor did that anyway, and now I was explicitly > using the constructor. But of course, this is totally wrong, because the > constructor would only run if mTxnMgr was false! So the first time > something called EnableUndo(true), it would create a new > nsTransactionManager with its max transaction count correctly initialized to > -1. But if something then called EnableUndo(false) and then > EnableUndo(true), such as by toggling designMode on and off, the max > transaction count would remain stuck at 0, stopping any undos or redos. Ah, dammit, I saw this when I was reviewing, and then I convinced myself the exact same way that you did. :(
Callek wasn't sure an m-i push now would make it into the uplift tomorrow, so I pushed straight to m-c: https://hg.mozilla.org/mozilla-central/rev/57abb5f70e01 He volunteered to watch the tree for me.
Big thanks to all of You who got involved in fixing this. It works as expected now. May the source be with You. Regards. Andrzej
or
