Bug 193706 - Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
Summary: Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: HTML Editing (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Wenson Hsieh
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-01-22 20:16 PST by Wenson Hsieh
Modified: 2019-01-24 19:03 PST (History)
9 users (show)

See Also:

Attachments
Patch (28.21 KB, patch)
2019-01-22 22:34 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Patch (27.94 KB, patch)
2019-01-22 22:49 PST, Wenson Hsieh 		rniwa: review+
Details | Formatted Diff | Diff
Review feedback & fix GTK build (19.15 KB, patch)
2019-01-23 07:48 PST, Wenson Hsieh 		no flags Details | Formatted Diff | Diff
Show Obsolete (1) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Wenson Hsieh 2019-01-22 20:16:32 PST 
More work towards <rdar://problem/44807048>.
Comment 1 Ryosuke Niwa 2019-01-22 22:30:42 PST 
I think we can just call it label() since the class is called UndoStep.
Comment 2 Wenson Hsieh 2019-01-22 22:34:42 PST 
Created attachment 359856 [details]
Patch
Comment 3 Wenson Hsieh 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 :/
Comment 4 Wenson Hsieh 2019-01-22 22:49:56 PST 
Created attachment 359857 [details]
Patch
Comment 5 Ryosuke Niwa 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.
Comment 6 Wenson Hsieh 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.
Comment 7 Wenson Hsieh 2019-01-23 07:48:35 PST 
Created attachment 359882 [details]
Review feedback & fix GTK build
Comment 8 WebKit Commit Bot 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>
Comment 9 Simon Fraser (smfr) 2019-01-23 20:44:07 PST 
Is the undo label controllable by the web content?
Comment 10 Wenson Hsieh 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).
Comment 11 Ryosuke Niwa 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.