[webkit-reviews] review granted: [Bug 87908] Add UNDO_MANAGER build flag : [Attachment 146591] Patch

[bugzilla-daemon at webkit.org]

Tony Chang <tony at chromium.org> has granted Sukolsak Sakshuwong
<sukolsak at google.com>'s request for review:
Bug 87908: Add UNDO_MANAGER build flag
https://bugs.webkit.org/show_bug.cgi?id=87908

Attachment 146591: Patch
https://bugs.webkit.org/attachment.cgi?id=146591&action=review

------- Additional Comments from Tony Chang <tony at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=146591&action=review


Did you want to be able to build using a command line flag (e.g., build-webkit
--enable-undo-manager)?  That code is in
Tools/Scripts/webkitperl/FeatureList.pm.

> Source/WebCore/dom/Document.cpp:217
> +#if ENABLE(UNDO_MANAGER)
> +#include "UndoManager.h"
> +#endif

Nit: I think it's OK to include this without the ENABLE check because the
header also does the check.  Other features don't seem to follow this pattern,
so I guess either style is OK.

> LayoutTests/platform/win/Skipped:1807
> +# UndoManager is not yet enabled.

Nit: Maybe include a link to the bug in the comment.

> LayoutTests/platform/wincairo/Skipped:2113
> +# UndoManager is not yet enabled.

Nit: Maybe include a link to the bug in the comment.