User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:9.0a1) Gecko/20110903 Firefox/9.0a1 Build ID: 20110903030832 Steps to reproduce: Pressed Ctrl+Z after opening a file in Scratchpad. Actual results: 1. Open Scratchpad 2. Note the placeholder text 3. Open any file (File -> Open File...) in Scratchpad 4. Select Edit -> Undo 5. Note that the placeholder text is back Expected results: The "undo/redo" stack should have been reset upon the new file being opened.

It would be nice if there was a simple reset() method set up for the Orion and textarea editors. I'm currently doing: "aWin.Scratchpad.editor._undoStack.reset()" for Orion, and "aWin.Scratchpad.editor._editor.resetModificationCount(); aWin.Scratchpad.editor_editor.transactionManager.clear()" for textarea Not pretty...

Thank you for the bug report! That is a good point, we need to clear the undo stack / transaction manager across file loads in Scratchpad, and the SourceEditor needs to provide a method to do that.

Adding a resetUndo function to both the textarea and orion and calling when a file successfully loads.

This bug say's it's for window 7 but it should repro on all platforms. I was able to verify on windows and linux. I'm not able to make that edit.

Thanks for your contribution Kenny! Much appreciated! I have assigned the bug to you now and I will review your patch as soon as possible.

Comment on attachment 581808 [details] [diff] [review] Add reset for undo stack Review of attachment 581808 [details] [diff] [review]: ----------------------------------------------------------------- Kenny, this patch applied cleanly and it does exactly what it needs to do. Really great work! Surgical patch that does pretty much what I had in mind. The next step is writing a test for the new Source Editor API and another test for Scratchpad, to check that file opens clear the undo stack. For further information on running and writing tests please read: https://developer.mozilla.org/en/Browser_chrome_tests The Source Editor tests live in browser/devtools/sourceeditor/test/ and the Scratchpad tests live in browser/devtools/scratchpad/test. Please copy in each folder one of the existing tests (the ones you see simplest / most fit for your testing purposes). Give the new tests file names that match the naming pattern of existing test files. Then edit each file as needed. For the Source Editor test I recommend making some text changes then undo, redo, check after each step if the editor content is the expected one. Then do resetUndo() and check again. Make more changes, then check state again. In the Source Editor _initialization.js file you should find sufficient code for inspiration. For the Scratchpad test I recommend you create two temporary files, fileA and fileB. open a Scratchpad, load fileA, make some changes, test undo/redo, check state, then open fileB, and check undo/redo states, see if the history was cleared. There's a _files.js test that has code working with files and with Scratchpad - you can use that for inspiration. For functions and methods you don't know about, please search MDN - there's a lot of cool and helpful documentation there. ;) Thank you very much for your contribution - much appreciated. (PS. on irc.mozilla.com you can join #devtools and talk to us!)

Here are the tests. Sorry I was not able to get the code and tests into one patch, I use Perforce at my work and I'm just learning Mercurial. Asked on #devtools but I guess no one was around. Let me know if there are further changes that need to be made.

Sorry for the noise. I was able to get some help using MQ to put all the code into one patch.

Comment on attachment 582606 [details] [diff] [review] reset undo and chrome tests Review of attachment 582606 [details] [diff] [review]: ----------------------------------------------------------------- This is really good work. Thank you Kenny! This patch is almost good to go, unfortunately you need to rebase it to the latest version of the code. Please do that - there are only minor changes to be made in the Scratchpad test. General nit-pick: the patch has trailing whitespace - please remove it. Thank you very much for your outright awesome contribution! Looking forward for the updated patch! ::: browser/devtools/scratchpad/test/browser_scratchpad_bug684546_reset_undo.js @@ +31,5 @@ > + gBrowser.selectedBrowser.removeEventListener("load", arguments.callee, true); > + > + gScratchpadWindow = Scratchpad.openScratchpad(); > + gScratchpadWindow.addEventListener("load", runTests, false); > + }, true); There have been changed in the fx-team repository that cause this test to fail now. Please update your repository to the latest code. Look into the updated tests and you'll see that the opening of scratchpads has changed a bit. ::: browser/devtools/sourceeditor/test/browser_bug684546_reset_undo.js @@ +32,5 @@ > +{ > + box = testWin.document.querySelector("box"); > + > + editorType = SpecialPowers.getCharPref("devtools.editor.component"); > + SpecialPowers.setCharPref("devtools.editor.component", "textarea"); Please test only the default editor component. Changing the component during runtime doesn't work. Once the Source Editor is loaded, subsequent changes of the preference have no impact - due to the way JSM loading works.

I applied all your feedback * rebased * removed all whitespace * changed the way I open the scratch pad * now only testing the default editor * double checked I had the lastest files before creating the patch * built and ran all devtools tests This patch was created from hg diff on my workspace instead of a committed changeset. It seems easier if I need to make additional changes. I hope this is ok as it's missing some of the changeset description information. I've found different wiki pages giving different instructions on creating patches.

Comment on attachment 582729 [details] [diff] [review] re-based and applied feedback to earlyer patch Review of attachment 582729 [details] [diff] [review]: ----------------------------------------------------------------- Patch looks really good. Thank you! Only some nits remain, see the comments below. ::: browser/devtools/scratchpad/test/browser_scratchpad_bug684546_reset_undo.js @@ +27,5 @@ > + waitForExplicitFinish(); > + > + gBrowser.selectedTab = gBrowser.addTab(); > + gBrowser.selectedBrowser.addEventListener("load", function() { > + gBrowser.selectedBrowser.removeEventListener("load", arguments.callee, true); arguments.callee is deprecated, please name the function. @@ +36,5 @@ > +} > + > +function runTests() > +{ > + gScratchpadWindow.removeEventListener("load", arguments.callee, false); This line is no longer needed once you changed to the new way of opening scratchpads. ::: browser/devtools/sourceeditor/test/browser_bug684546_reset_undo.js @@ +14,5 @@ > + waitForExplicitFinish(); > + > + const windowUrl = "data:application/vnd.mozilla.xul+xml,<?xml version='1.0'?>" + > + "<window xmlns='http://www.mozilla.org/keymaster/gatekeeper/there.is.only.xul'" + > + " title='test for bug 687573 - vertical scroll' width='300' height='500'>" + Please update the window title. @@ +37,5 @@ > +function editorLoaded() > +{ > + editor.setText("First"); > + editor.setText("Second", 5); > + is(editor.getText(), "FirstSecond", "textarea: Text set correctly."); The "textarea: " prefix is no longer needed in these tests.

(In reply to Kenny Heaton from comment #10) > Created attachment 582729 [details] [diff] [review] > re-based and applied feedback to earlyer patch > > I applied all your feedback > * rebased > * removed all whitespace > * changed the way I open the scratch pad > * now only testing the default editor > * double checked I had the lastest files before creating the patch > * built and ran all devtools tests > > This patch was created from hg diff on my workspace instead of a committed > changeset. It seems easier if I need to make additional changes. I hope this > is ok as it's missing some of the changeset description information. I've > found different wiki pages giving different instructions on creating patches. To improve your workflow I recommend you use mercurial queues. See: https://developer.mozilla.org/en/Mercurial_Queues http://mercurial.selenic.com/wiki/MqExtension http://mercurial.selenic.com/wiki/MqTutorial General Mercurial info: https://developer.mozilla.org/en/Mercurial_FAQ Good luck and thanks for your well-executed contribution!

Comment on attachment 583069 [details] [diff] [review] [in-fx-team] Applied feedback to earlyer patch Review of attachment 583069 [details] [diff] [review]: ----------------------------------------------------------------- This is ready to land! Awesome work! Thank you Kenny!

Thanks for all your help. Since I don't have push privileges will you be able to push this patch for me?

(In reply to Kenny Heaton from comment #15) > Thanks for all your help. Since I don't have push privileges will you be > able to push this patch for me? Yep, that's the plan. I will land your patch, hopefully today. Thanks!

Comment on attachment 583069 [details] [diff] [review] [in-fx-team] Applied feedback to earlyer patch Landed: https://hg.mozilla.org/integration/fx-team/rev/a98e349f29f7 Kenny, thank you for this high quality patch!

https://hg.mozilla.org/mozilla-central/rev/a98e349f29f7 Nice work Kenny! :-)

The addition of resetUndo() to the Source Editor API in Firefox 12 should be documented. Thank you!

resetUndo() method documented: Listed on Firefox 12 for developers.