Bug 193706 – Introduce UndoStep::label() and adopt it in WebKitLegacy and WebKit

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Wenson Hsieh]

More work towards <rdar://problem/44807048>.

[Ryosuke Niwa]

I think we can just call it label() since the class is called UndoStep.

[Wenson Hsieh]

Created attachment 359856 [details]
Patch

[Wenson Hsieh]

(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]

Created attachment 359857 [details]
Patch

[Ryosuke Niwa]

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 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]

Created attachment 359882 [details]
Review feedback & fix GTK build

[WebKit Commit Bot]

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)]

Is the undo label controllable by the web content?

[Wenson Hsieh]

(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]

(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.