Add support for undo/redo to keyboard API

RESOLVED INCOMPLETE

Status

()

Core
DOM: Device Interfaces
RESOLVED INCOMPLETE
4 years ago
5 months ago

People

(Reporter: sicking, Unassigned)

Tracking

unspecified
x86
Mac OS X
Points:
---

Firefox Tracking Flags

(tracking-b2g:backlog)

Details

(Whiteboard: [FT:System-Platform])

It's very annoying that it's not possible to undo if you accidentally delete too much text. We should make it possible for keyboards to have an 'undo' and 'redo' button.

For now adding simple commands to call sendUndo() and sendRedo() on the InputContext should be enough. Potentially we could also enable ways for the keyboard to know if undo/redo is possible. I.e. if the editor is currently at the beginning or end of the "undo history".

Gecko already has undo/redo capability in our editors, so simply hooking in to that should do the trick.
Why |sendUndo| and not |undo|?
I was just monitoring sendKey(). These functions should undo/redo any actions made to the text field, not just actions that are coming through the keyboard. So it felt more like sending an undo command, than simply doing an undo.

But either name is totally fine with me.

Comment 3

4 years ago
Let me know if you need help hooking this up to the editor's transaction manager.
How about this?

partial interface InputContext {
  Promise<void> undo();
  Promise<void> redo();
  readonly attribute boolean canUndo;
  readonly attribute boolean canRedo;
};
Sounds good, should be pretty easy to implement.

Comment 6

4 years ago
(In reply to comment #4)
> How about this?
> 
> partial interface InputContext {
>   Promise<void> undo();
>   Promise<void> redo();
>   readonly attribute boolean canUndo;
>   readonly attribute boolean canRedo;
> };

Three questions:

1. Does it make sense for us to try to undo/redo multiple transactions in one go?  The internal nsIEditor API allows for that.
2. On the editor side, we have two reasons why undo and redo cannot be performed:
a) Because the transaction manager is disabled for that editor, and b) because there are no transactions to be undone/redone.  It seems to me like the canUndo and canRedo attributes are meant for the latter.  Do we also need to worry about the former?
3. Why are canUndo and canRedo synchronous?  I think that means that we would need to block on an IPC message, which is not great.
> 1. Does it make sense for us to try to undo/redo multiple transactions in
> one go?  The internal nsIEditor API allows for that.

"what's the use case?"

> 2. On the editor side, we have two reasons why undo and redo cannot be
> performed:
> a) Because the transaction manager is disabled for that editor, and b)
> because there are no transactions to be undone/redone.  It seems to me like
> the canUndo and canRedo attributes are meant for the latter.  Do we also
> need to worry about the former?

this seems like something we should ask someone that knows the editor code ;-)

is the transaction manager ever disabled for web content?

> 3. Why are canUndo and canRedo synchronous?  I think that means that we
> would need to block on an IPC message, which is not great.

sync is always better if we can do it without blocking IPC. Can we here? I.e. can we transfer the state whoever we transfer information about changes to the editable text? When do these values change?
(In reply to Jonas Sicking (:sicking) needinfo? or feedback? encouraged. from comment #7)
> > 3. Why are canUndo and canRedo synchronous?  I think that means that we
> > would need to block on an IPC message, which is not great.
> 
> sync is always better if we can do it without blocking IPC. Can we here?
> I.e. can we transfer the state whoever we transfer information about changes
> to the editable text? When do these values change?

It is possible and we may add an event handler to InputContext to notify about the changes:

partial interface InputContext : EventTarget{
  Promise<void> undo();
  Promise<void> redo();
  readonly attribute boolean canUndo;
  readonly attribute boolean canRedo;
  attribute EventHandler onundoreduchange;
};

We can transfer the states of `canUndo` and `canRedu` when they change and fire "undo redo change" event (is there a better name?) to the keyboard.

Comment 9

4 years ago
(In reply to Jonas Sicking (:sicking) needinfo? or feedback? encouraged. from comment #7)
> > 1. Does it make sense for us to try to undo/redo multiple transactions in
> > one go?  The internal nsIEditor API allows for that.
> 
> "what's the use case?"

That's what I was asking.  :-)

> > 2. On the editor side, we have two reasons why undo and redo cannot be
> > performed:
> > a) Because the transaction manager is disabled for that editor, and b)
> > because there are no transactions to be undone/redone.  It seems to me like
> > the canUndo and canRedo attributes are meant for the latter.  Do we also
> > need to worry about the former?
> 
> this seems like something we should ask someone that knows the editor code
> ;-)
> 
> is the transaction manager ever disabled for web content?

Yes, I should have been more explicit here.  We turn off the transaction manager for password input boxes.

> > 3. Why are canUndo and canRedo synchronous?  I think that means that we
> > would need to block on an IPC message, which is not great.
> 
> sync is always better if we can do it without blocking IPC. Can we here?
> I.e. can we transfer the state whoever we transfer information about changes
> to the editable text? When do these values change?

Hmm, I _think_ we should be able to transfer that information in the same message.  But somebody more familiar with the keyboard code should confirm.
blocking-b2g: --- → backlog
Whiteboard: [FT:System-Platform]
(Assignee)

Updated

3 years ago
blocking-b2g: backlog → ---
tracking-b2g: --- → backlog
FxOS/Gonk has been removed from the codebase. Mass-invalidating FxOS related Device Interface bugs to clean up the component. 

If I incorrectly invalidated something, please let me know.
Status: NEW → RESOLVED
Last Resolved: 5 months ago
Resolution: --- → INVALID
Bulk correction of resolution of B2G bugs to INCOMPLETE.
Resolution: INVALID → INCOMPLETE
You need to log in before you can comment on or make changes to this bug.