[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