[webkit-reviews] review denied: [Bug 26573] [Gtk] Add Undo/Redo support to WebKitGtk : [Attachment 31651] revise the patch to use the wtf/Deque.h
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Jun 24 03:20:42 PDT 2009
Jan Alonzo <jmalonzo at gmail.com> has denied Jiahua Huang
<jhuangjiahua at gmail.com>'s request for review:
Bug 26573: [Gtk] Add Undo/Redo support to WebKitGtk
https://bugs.webkit.org/show_bug.cgi?id=26573
Attachment 31651: revise the patch to use the wtf/Deque.h
https://bugs.webkit.org/attachment.cgi?id=31651&action=review
------- Additional Comments from Jan Alonzo <jmalonzo at gmail.com>
> +void
EditorClient::registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand>
command)
> {
> - notImplemented();
> + if (undoStack.size() == maximumUndoStackDepth)
> + undoStack.removeFirst();
Kindly fix the spacing.
> bool EditorClient::shouldInsertNode(Node*, Range*, EditorInsertAction)
> @@ -524,6 +547,7 @@ EditorClient::EditorClient(WebKitWebView
> : m_webView(webView)
> {
> WebKitWebViewPrivate* priv = m_webView->priv;
> + isInRedo = false;
This needs to be in the initialization list.
> class EditorClient : public WebCore::EditorClient {
> + protected:
> + bool isInRedo;
Member variables need to be prefixed with m_. See coding style for details.
> +
> + typedef WTF::Deque<WTF::RefPtr<WebCore::EditCommand> >
EditCommandStack;
> + EditCommandStack undoStack;
> + EditCommandStack redoStack;
Is the typedef necessary since you're only using EditCommandStack in these two
lines?
r- for now until those the style issues are fixed. Patch looks fine otherwise.
More information about the webkit-reviews
mailing list