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

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.