[Webkit-unassigned] [Bug 26573] [Gtk] Add Undo/Redo support to WebKitGtk

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=26573


zecke at selfish.org changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #31605|review?                     |review-
               Flag|                            |




------- Comment #9 from zecke at selfish.org  2009-06-21 23:50 PDT -------
(From update of attachment 31605)
First of all great you are working on it, and I assumed that such a
implementation would be much bigger than what you came up with...

A general remark is please read:
    http://webkit.org/coding/contributing.html
    http://webkit.org/coding/coding-style.html


> Index: WebKit/gtk/ChangeLog
> ===================================================================
> --- WebKit/gtk/ChangeLog	(revision 44907)
> +++ WebKit/gtk/ChangeLog	(working copy)
> @@ -1,3 +1,18 @@
> +2009-06-21  Jiahua Huang  <jhuangjiahua at gmail.com>
> +
> +        Unreviewed. Add Undo/Redo support to WebKitGtk.

Please us the prepare-ChangeLog tool to generate the changelog.


> +// Arbitrary depth limit for the undo stack, to keep it from using
> +// unbounded memory.  This is the maximum number of distinct undoable
> +// actions -- unbroken stretches of typed characters are coalesced
> +// into a single action.
> +static const size_t kMaximumUndoStackDepth = 1000;


the naming of the variable is a bit awkward... we don't use the 'k' for
konstants or such... just make it maximumUndoStackDepth, or to reduce the
binary size make it a #define..


>  bool EditorClient::shouldInsertNode(Node*, Range*, EditorInsertAction)
> Index: WebKit/gtk/WebCoreSupport/EditorClientGtk.h
> ===================================================================
> --- WebKit/gtk/WebCoreSupport/EditorClientGtk.h	(revision 44907)

> +#include <deque>
> +

Main reason to say know right now. I don't think we want to have more stl
dependencies. Could you please revise the patch to use the wtf/Deque.h and
check if that works?


>  #include "EditorClient.h"
>  
>  #include <wtf/Forward.h>
> @@ -43,6 +45,13 @@ namespace WebCore {
>  namespace WebKit {
>  
>      class EditorClient : public WebCore::EditorClient {
> +    protected:
> +        bool isInRedo;


To make the paranoid happy, could you please add an EditorClient constructor
and initialize isInRedo?


-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug, or are watching the assignee.