683172 - Source Editor should automatically set up Undo/Redo key bindings

Status: RESOLVED FIXED

(DevTools :: General, defect, P3)

Description

[Cedric Vivier [:cedricv]]

Of the following basic editing features that should be default:

cut/copy/past/select all/undo/redo  

The two latter (Undo and Redo) are not supported automatically by SourceEditor currently.
Orion Key bindings should probably be registered in _initEditorFeatures... it might be worth to expose a disableEditorFeatures boolean to config object so that a consumer of SourceEditor can get full control if needed.

Comment 1

[Cedric Vivier [:cedricv]]

Attachment: Source Editor should automatically set up Undo/Redo key bindings - b=683172 r=msucan

Comment 2

[Cedric Vivier [:cedricv]]

Attachment: Source Editor should automatically set up Undo/Redo key bindings - b=683172 r=msucan

Comment 3

[Mihai Sucan [:msucan]]

Comment on attachment 556868 [details] [diff] [review]
Source Editor should automatically set up Undo/Redo key bindings - b=683172 r=msucan

Thanks for your bug report and quick patch! Quick comments:

- key bindings need to be localizable.
- please use "undo" and "redo" as action names (standard in upstream Orion).
- we need a test as well.
- please update scratchpad accordingly. I assume you need to remove the <key> xul elements from scratchpad.xul.

Giving r- for now. Looking forward for the updated patch. Thanks again!

Comment 4

[Mihai Sucan [:msucan]]

Not sure, but maybe we need to use the system default undo/redo keys. Rob, any thoughts?

Comment 5

[Rob Campbell [:rc] (:robcee)]

System default, you mean OS defaults or Gecko defaults? If you mean Gecko defaults for whichever locale it's in, then yes.

Comment 6

[Rob Campbell [:rc] (:robcee)]

little book-keeping here so we know who's working on this.

Comment 7

[Mihai Sucan [:msucan]]

Rob: I meant Gecko defaults.

Comment 8

[Rob Campbell [:rc] (:robcee)]

yes, should use gecko defaults.

Comment 9

[Dave Camp (:dcamp)]

We're doing developer tool prioritization, filter on 'brontozaur'
to ignore the spam.

Comment 10

[Mihai Sucan [:msucan]]

Attachment: proposed patch

Proposed patch. Things to note:

- I looked into how I can use the Gecko default keybindings and it seems that the textarea implementation is pretty much hard coded (ctrl/cmd-z and ctrl/cmd-shift-z).

- in scratchpad.xul we need to keep the <key> element, so we can have the shortcuts visible in the menus.

Please let me know if this is fine. Looking forward to your review. Thanks!


I looked into adding an nsIController for the SourceEditor, but it didn't work well, so I did bail out for now. It would be "over-engineered" for this bug alone. Anyhow, I think it might make sense, for the future, to have an nsIController that calls the Orion API for the most common cmd_* stuff. Is this worth a separate bug, what do you think?


(Cedric has agreed with me taking this bug.)

Comment 11

[Mihai Sucan [:msucan]]

Thank you Rob!

Comment 12

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/integration/fx-team/rev/c135571daddf

Comment 13

[Rob Campbell [:rc] (:robcee)]

https://hg.mozilla.org/mozilla-central/rev/c135571daddf

Comment 14

[Mihai Sucan [:msucan]]

The addition of the default Undo/Redo keyboard shortcuts in Firefox 11 to the Source Editor should be documented. Thank you!

Comment 15

[Eric Shepherd [:sheppy]]

Documented:

https://developer.mozilla.org/en/Tools/Using_the_Source_Editor#Keyboard_commands