Bug 193706

Summary: Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
Product: WebKit Reporter: Wenson Hsieh <wenson_hsieh>
Component: HTML EditingAssignee: Wenson Hsieh <wenson_hsieh>
Status: RESOLVED FIXED    
Severity: Normal CC: aestes, bdakin, commit-queue, megan_gardner, rniwa, simon.fraser, thorton, webkit-bug-importer, wenson_hsieh
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
rniwa: review+
Review feedback & fix GTK build none

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
DetailsFormatted DiffDiff
Patch (27.94 KB, patch)
2019-01-22 22:49 PST, Wenson Hsieh 		rniwa: review+
DetailsFormatted DiffDiff
Review feedback & fix GTK build (19.15 KB, patch)
2019-01-23 07:48 PST, Wenson Hsieh 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
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
Created attachment 359856 [details] Patch
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
Created attachment 359857 [details] Patch
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.