Need a way for JavaScript (or bundle) code to participate in undo

• Component: HTML Editing
• Status: RESOLVED FIXED

There needs to be a way for code in the Web Content process—presumably JavaScript code on the page, but possibly bundle code—to participate in undo.

Here is a concrete use case that should be supported: if an (undoable) editing operation triggers an event handler (such as an input event handler or DOM mutation event handler), then in the context of handling the event, there should be a way to register a handler to be run when the editing operation gets undone. There should probably also be a way to determine whether the event handler is running because of an undo/redo invocation.

<rdar://problem/44807048>

[Ryosuke Niwa]

See https://bugs.webkit.org/show_bug.cgi?id=87189 for the previous attempt.

[Wenson Hsieh]

*** Bug 193720 has been marked as a duplicate of this bug. ***

[Wenson Hsieh]

Created attachment 359897 [details]
Patch

[Wenson Hsieh]

Created attachment 359905 [details]
Fix WPE/GTK/Win

[EWS Watchlist]

Comment on attachment 359905 [details]
Fix WPE/GTK/Win

Attachment 359905 [details] did not pass mac-wk2-ews (mac-wk2):
Output: https://webkit-queues.webkit.org/results/10861078

New failing tests:
imported/w3c/web-platform-tests/css/css-grid/abspos/orthogonal-positioned-grid-descendants-015.html

[EWS Watchlist]

Created attachment 359919 [details]
Archive of layout-test-results from ews106 for mac-highsierra-wk2

The attached test failures were seen while running run-webkit-tests on the mac-wk2-ews.
Bot: ews106  Port: mac-highsierra-wk2  Platform: Mac OS X 10.13.6

[Ryosuke Niwa]

Comment on attachment 359905 [details]
Fix WPE/GTK/Win

View in context: https://bugs.webkit.org/attachment.cgi?id=359905&action=review

> Source/WebCore/editing/CompositeEditCommand.h:92
> +    void invalidate() final { }

I don't like "invalidate". It doesn't tell us what it does, nor what we're invalidating, or what invalidating edit command means.
I think we should say something more specific like didRemoveFromUndoManager.

> Source/WebCore/page/UndoManager.cpp:48
> +        // FIXME: Support adding the same UndoItem to an UndoManager multiple times.

We probably want to throw in this case.

> Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:299
> +    readonly attribute DOMString undoActionName;
> +    readonly attribute DOMString redoActionName;

action name is Cocoa terminology. It's also somewhat surprising that there is only one name.
We should probably call these something like lastUndoLabel and firstRedoLabel.

[Wenson Hsieh]

Comment on attachment 359905 [details]
Fix WPE/GTK/Win

View in context: https://bugs.webkit.org/attachment.cgi?id=359905&action=review

>> Source/WebCore/editing/CompositeEditCommand.h:92
>> +    void invalidate() final { }
> 
> I don't like "invalidate". It doesn't tell us what it does, nor what we're invalidating, or what invalidating edit command means.
> I think we should say something more specific like didRemoveFromUndoManager.

Ok! Renamed this to didRemoveFromUndoManager().

>> Source/WebCore/page/UndoManager.cpp:48
>> +        // FIXME: Support adding the same UndoItem to an UndoManager multiple times.
> 
> We probably want to throw in this case.

Good call. I made UndoManager.addItem() capable of throwing exceptions, and added exceptions in the case of this early return, and the !frame check below. This also made me realize that this first check for `m_items.contains(item.ptr())` isn't quite right — we really want to bail if the given UndoItem already has already been added an undoManager, not just this undo manager in particular!

I added a new layout test as well, to verify that we throw an exception.

>> Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:299
>> +    readonly attribute DOMString redoActionName;
> 
> action name is Cocoa terminology. It's also somewhat surprising that there is only one name.
> We should probably call these something like lastUndoLabel and firstRedoLabel.

Sounds good! Renamed to lastUndoLabel and firstRedoLabel.

[Wenson Hsieh]

Created attachment 360113 [details]
Patch for EWS

[Wenson Hsieh]

Created attachment 360117 [details]
Patch for EWS

[WebKit Commit Bot]

Comment on attachment 360117 [details]
Patch for EWS

Clearing flags on attachment: 360117

Committed r240476: <https://trac.webkit.org/changeset/240476>