[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





------- Comment #6 from jmalonzo at gmail.com  2009-06-20 15:10 PDT -------
(From update of attachment 31597)
Thanks for updating the patch.

> -void EditorClient::registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand>)
> +void EditorClient::registerCommandForUndo(WTF::PassRefPtr<WebCore::EditCommand> command)
>  {
> -    notImplemented();
> +    if (undo_stack_.size() == kMaximumUndoStackDepth)
> +        undo_stack_.pop_front();  // drop oldest item off the far end

The variable names need to be fixed - should be undoStack for example. Please
see the coding style guidelines I referenced earlier.

>  void EditorClient::undo()
>  {
> -    notImplemented();
> +    if (canUndo()) {
> +        RefPtr<WebCore::EditCommand> command(undo_stack_.back());
> +        undo_stack_.pop_back();
> +        command->unapply();
> +        // unapply will call us back to push this command onto the redo stack.

Please put the comment before the operation it's trying to give context to. Not
after.

>  void EditorClient::redo()
>  {
> -    notImplemented();
> +    if (canRedo()) {
> +        RefPtr<WebCore::EditCommand> command(redo_stack_.back());
> +        redo_stack_.pop_back();
> +
> +        ASSERT(!in_redo_);
> +        in_redo_ = true;
> +        command->reapply();
> +        // reapply will call us back to push this command onto the undo stack.

Ditto.

>      class EditorClient : public WebCore::EditorClient {
> +    protected:
> +        bool use_editor_delegate_;

Where is this variable used?

Looks fine nevertheless. Please mark the review flag as r? next time so it will
end up in the review queue and other reviewers can review it too.

Thanks!


-- 
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.