[webkit-reviews] review denied: [Bug 62637] [Qt] [WK2] Qt WebKit2 needs undo/redo support : [Attachment 97145] fix patch 2
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Jun 14 15:36:50 PDT 2011
Kenneth Rohde Christiansen <kenneth at webkit.org> has denied Chang Shu
<cshu at webkit.org>'s request for review:
Bug 62637: [Qt] [WK2] Qt WebKit2 needs undo/redo support
https://bugs.webkit.org/show_bug.cgi?id=62637
Attachment 97145: fix patch 2
https://bugs.webkit.org/attachment.cgi?id=97145&action=review
------- Additional Comments from Kenneth Rohde Christiansen
<kenneth at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=97145&action=review
> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:116
> + delete undoStack;
I agree that we should start using smart pointers. OwnPtr works quite well with
Qt types, just like QScopedPointer. delete should be a thing of the past or at
least you should explain why a smart pointer cannot be used
> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:226
> + undoStack->push(new WebUndoCommandQt(command));
Who are freeing these?
> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:234
> + undoStack->clear();
Will this free everything in the stack? That is not normal behavior in webcore
afaik so we might need a comment if that is Qt behavior
> Source/WebKit2/UIProcess/qt/WebUndoCommandQt.h:49
> + bool m_inUndoRedo; // our undo stack works differently - don't
re-enter!
Our*
Maybe a better variable name can be found.
More information about the webkit-reviews
mailing list