RESOLVED FIXED 193706
Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
https://bugs.webkit.org/show_bug.cgi?id=193706
Summary Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
Wenson Hsieh
Reported 2019-01-22 20:16:32 PST
More work towards <rdar://problem/44807048>.
Attachments
Patch (28.21 KB, patch)
2019-01-22 22:34 PST, Wenson Hsieh
no flags
Patch (27.94 KB, patch)
2019-01-22 22:49 PST, Wenson Hsieh
rniwa: review+
Review feedback & fix GTK build (19.15 KB, patch)
2019-01-23 07:48 PST, Wenson Hsieh
no flags
Ryosuke Niwa
Comment 1 2019-01-22 22:30:42 PST
I think we can just call it label() since the class is called UndoStep.
Wenson Hsieh
Comment 2 2019-01-22 22:34:42 PST
Wenson Hsieh
Comment 3 2019-01-22 22:35:28 PST
(In reply to Ryosuke Niwa from comment #1) > I think we can just call it label() since the class is called UndoStep. Oops, I just saw this now :/
Wenson Hsieh
Comment 4 2019-01-22 22:49:56 PST
Ryosuke Niwa
Comment 5 2019-01-23 00:31:43 PST
Comment on attachment 359857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359857&action=review > Source/WebKit/ChangeLog:45 > + Adjust this to take Ref<UndoStep>&&. Why are we making this change? > Source/WebKit/UIProcess/WebEditCommandProxy.h:42 > - static Ref<WebEditCommandProxy> create(WebUndoStepID commandID, WebCore::EditAction editAction, WebPageProxy* page) > + static Ref<WebEditCommandProxy> create(WebUndoStepID commandID, const String& label, WebPageProxy& page) Hm... I would have kept WebCore::EditAction for this one but okay.
Wenson Hsieh
Comment 6 2019-01-23 07:21:35 PST
Comment on attachment 359857 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=359857&action=review >> Source/WebKit/ChangeLog:45 >> + Adjust this to take Ref<UndoStep>&&. > > Why are we making this change? I thought it would be preferable to UndoStep&, but I can revert this part. >> Source/WebKit/UIProcess/WebEditCommandProxy.h:42 >> + static Ref<WebEditCommandProxy> create(WebUndoStepID commandID, const String& label, WebPageProxy& page) > > Hm... I would have kept WebCore::EditAction for this one but okay. I think it should be okay to remove editAction from WebEditCommandProxy, since the only thing we used it for was the undo/redo label.
Wenson Hsieh
Comment 7 2019-01-23 07:48:35 PST
Created attachment 359882 [details] Review feedback & fix GTK build
WebKit Commit Bot
Comment 8 2019-01-23 09:30:11 PST
Comment on attachment 359882 [details] Review feedback & fix GTK build Clearing flags on attachment: 359882 Committed r240342: <https://trac.webkit.org/changeset/240342>
Simon Fraser (smfr)
Comment 9 2019-01-23 20:44:07 PST
Is the undo label controllable by the web content?
Wenson Hsieh
Comment 10 2019-01-23 20:48:39 PST
(In reply to Simon Fraser (smfr) from comment #9) > Is the undo label controllable by the web content? Yes. That being said, the UndoManager API isn't web-exposed (at least, not currently).
Ryosuke Niwa
Comment 11 2019-01-24 19:03:18 PST
(In reply to Wenson Hsieh from comment #10) > (In reply to Simon Fraser (smfr) from comment #9) > > Is the undo label controllable by the web content? > > Yes. That being said, the UndoManager API isn't web-exposed (at least, not > currently). We probably need to do some sanitization if we decide to expose this API to the Web.
Note You need to log in before you can comment on or make changes to this bug.