[webkit-reviews] review granted: [Bug 191487] Implement a new edit command to change the enclosing list type : [Attachment 354485] Remove some debugging code.

[bugzilla-daemon at webkit.org]

Ryosuke Niwa <rniwa at webkit.org> has granted Wenson Hsieh
<wenson_hsieh at apple.com>'s request for review:
Bug 191487: Implement a new edit command to change the enclosing list type
https://bugs.webkit.org/show_bug.cgi?id=191487

Attachment 354485: Remove some debugging code.

https://bugs.webkit.org/attachment.cgi?id=354485&action=review




--- Comment #4 from Ryosuke Niwa <rniwa at webkit.org> ---
Comment on attachment 354485
  --> https://bugs.webkit.org/attachment.cgi?id=354485
Remove some debugging code.

View in context: https://bugs.webkit.org/attachment.cgi?id=354485&action=review

> Source/WebCore/editing/ChangeListTypeCommand.cpp:81
> +    RefPtr<HTMLElement> listToReplace;
> +    auto type = listConversionTypeForSelection(endingSelection(),
listToReplace);

Why don't we make this return both type & RefPtr using one time struct or pair?

> Tools/TestWebKitAPI/Tests/WebKitCocoa/WKWebViewEditActions.mm:129
> +    auto webView =
webViewForEditActionTestingWithPageNamed(@"editable-nested-lists");
> +
> +    [webView setPosition:@"one" offset:1];
> +    [webView _changeListType:nil];
> +    EXPECT_TRUE([webView querySelectorExists:@"ol > li#one.item"]);

I think we should add a layout test instead.
In general, we should be writing layout tests over API tests whenever possible.