We need to allow developers to do search and replace operations in the source code, within Workspace.
This feature needs to be supported in the Source Editor.
Going to morph this bug into: - add a minimal UI for find/find next/find previous (with a prompt() as agreed on IRC) - add a minimal UI for jump to line (we already have the API in bug 687160). Both are trivial to do and they'd be really nice usability wins for Firefox 11. Will open a follow up bug to improve the UI for these two features. I will also open a follow up to get find *and* replace support in the Source Editor component. This will fix bug 710925.
Created attachment 582318 [details] [diff] [review] proposed patch (wip) Proposed patch, no tests yet. Functionality should be complete. Notes on the code: - I added a JSM to keep UI stuff away from the main "text editor widget" code. I expect the UI stuff will grow to be much bigger when we add more and more features. Also, SourceEditorUI is made reusable with any SourceEditor component. - I did put some generic code in the main source-editor.jsm that implements features that use the public Source Editor APIs. This is code that is reusable with any component. This is the case with find/findNext/findPrevious, etc. Please let me know if you have comments on how things stand. Thank you! (I will add tests ASAP.)
Created attachment 582550 [details] [diff] [review] proposed patch Updated patch, with fixes and a test for the new functionality. I changed the keyboard shortcuts to avoid collisions with existing shortcuts.
Created attachment 582551 [details] [diff] [review] only the changes since yesterday To make reviewing easier, here are the changes I did since yesterday: fixes and the new test.
Comment on attachment 582551 [details] [diff] [review] only the changes since yesterday I don't think we can hard-code these values. I also think they need to be different on each platform. DOM_VK_F is probably fine for "Find" on most platforms. DOM_VK_F3 makes sense on Windows, though not on mac which uses DOM_VK_G for find next. Why DOM_VK_G for go to line? I would have thought L would be a better choice? (I guess "Go to line"). Unfortunately, I don't think we can do this without making the keys localizeable. Either a properties file or figure out how to wire up commands and keys in xul.
(In reply to Rob Campbell [:rc] (robcee) from comment #8) > Why DOM_VK_G for go to line? I would have thought L would be a better > choice? (I guess "Go to line"). In view-source we use ctrl-l.
looking at this again, I don't think we can use F3 on Mac. It needs to be Cmd-G for Find Next, which means we need an alternate key for Goto Line.
Created attachment 585548 [details] [diff] [review] updated patch Updated the patch. Made the keys localizable. I also moved out the code for the Jump to line UI, see bug 714942, as requested. I'd like a decision on which keyboard shortcuts to use for find, find next and find previous. If needed, we can make different shortcuts for each OS. Please let me know. Thanks! Also, I think this is the place where I should make the change for bug 714832. Shall I do it here in this patch? I already touch the relevant lines of code. Looking forward for your review. Thank you! (apologies for the time it took to update the patch - holidays took over!)
Created attachment 585807 [details] [diff] [review] patch update 3 Updated to use the keyboard shortcuts we decided on IRC: https://etherpad.mozilla.org/x0WpxA9bcm Thank you!
Created attachment 588492 [details] [diff] [review] patch update 4 Updated based on Dão's comments in bug 714942. This UI is now only available in Scratchpad. For the Style Editor and for the JS debugger we should open separate bugs. For example, the Style Editor needs a menu (afaik there's a bug on that, can't remember the number now). Please let me know if further changes are needed. Thank you!
Comment on attachment 588492 [details] [diff] [review] patch update 4 in scratchpad.xul + <key id="key_findPrevious" + key="&findPreviousCmd.key;" + command="cmd_findPrevious" + modifiers="accel"/> should be modifiers="accel,shift" This looks very nice. R+ with a successful run through try and the above change.
Comment on attachment 588492 [details] [diff] [review] patch update 4 in source-editor-orion.jsm + "Find...": [this.ui.find, this.ui], do these show up anywhere visible? if so, you should use a real ellipsis instead of the '...'. i.e., '…'.
(In reply to Rob Campbell [:rc] (robcee) from comment #14) > Comment on attachment 588492 [details] [diff] [review] > patch update 4 > > in scratchpad.xul > > + <key id="key_findPrevious" > + key="&findPreviousCmd.key;" > + command="cmd_findPrevious" > + modifiers="accel"/> > > should be modifiers="accel,shift" True. I forgot this when I wrote this part of the code. I actually had test failures on Macs because of this small issue. Fixed. > This looks very nice. R+ with a successful run through try and the above > change. Thanks! I did push the fixed patch to the try server. Green results: https://tbpl.mozilla.org/?tree=Try&rev=b711b368e4b1 (In reply to Rob Campbell [:rc] (robcee) from comment #15) > Comment on attachment 588492 [details] [diff] [review] > patch update 4 > > in source-editor-orion.jsm > > + "Find...": [this.ui.find, this.ui], > > do these show up anywhere visible? No. These are names for Orion actions, they are part of the Orion API stuff. The naming is chosen to match common action names from upstream. Thanks for checking on this - it's a valid concern, but luckily these names are never displayed.
Created attachment 589249 [details] [diff] [review] [in-fx-team] patch update 5 Fixed the keyboard shortcut.
Comment on attachment 589249 [details] [diff] [review] [in-fx-team] patch update 5 https://hg.mozilla.org/integration/fx-team/rev/5104705ee96a
Please document the addition of the following in Firefox 12: - new source-editor-overlay.xul that needs to be included in the XUL where SourceEditor is to be used. - <commandset id="sourceEditorCommands"/> also needs to be added into the XUL document, to make the SourceEditor commands available. - new SourceEditorUI JSM that provides a common UI to any Source Editor component. - this has its own nsIController (the SourceEditorController) that implements common editor commands. For now it has cmd_find, cmd_findAgain and cmd_findPrevious. - the SourceEditorUI is not generally meant to be used by any other extension or code, except those extensions that write new SourceEditor components. To use the UI methods one must have a good reason. - the UI has methods to invoke the find/find next/find previous actions. - to invoke the commands use the global commands, for example do goDoCommand("cmd_find"). This call is handled by the SourceEditorController which passes the request to the SourceEditorUI that in turn uses the SourceEditor component to perform the find. - new methods for the SourceEditor instances: find(), findNext() and findPrevious(). - new properties for the SourceEditor instances: ui and lastFind. See source-editor.jsm. - new keyboard shortcuts: Ctrl-F (Find), F3 (Find next on Windows/Linux), Shift-F3 (Find previous on Windows/Linux), Ctrl-G (Find next on Macs), Ctrl-Shift-G (Find previous on Macs). - and anything I missed to mention now. ;) For usage examples please see the new tests and the changes in scratchpad.js and scratchpad.xul. Thank you!