Bug 62809 – [Qt] [WK2] Support undo/redo in MiniBrowser

• Component: Tools / Tests
• Status: RESOLVED FIXED

[Chang Shu]

Add UI support of undo/redo in MiniBrowser and WebKit2.

[Chang Shu]

Created attachment 97482 [details]
fix patch

[Yael]

Comment on attachment 97482 [details]
fix patch

View in context: https://bugs.webkit.org/attachment.cgi?id=97482&action=review

> Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:875
> +QUndoStack* QWKPage::undoStack() const

Who is using this new API?

[Chang Shu]

(In reply to comment #2)
> (From update of attachment 97482 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=97482&action=review
> > Source/WebKit2/UIProcess/API/qt/qwkpage.cpp:875
> > +QUndoStack* QWKPage::undoStack() const
> Who is using this new API?

Just in my patch. But it's true that we can remove it and use d->undoStack.

[Andreas Kling]

Comment on attachment 97482 [details]
fix patch

View in context: https://bugs.webkit.org/attachment.cgi?id=97482&action=review

> Source/WebKit2/UIProcess/API/qt/qwkpage.h:131
> +    bool isModified() const;
> +#ifndef QT_NO_UNDOSTACK
> +    QUndoStack* undoStack() const;
> +#endif

undoStack() should be replaced by d->undoStack inside QWKPage.
Why do we need isModified() and the 'modified' property?

> Tools/MiniBrowser/qt/BrowserWindow.cpp:89
> +    undo->setShortcut(QKeySequence(Qt::CTRL | Qt::Key_Z));

Use the QKeySequence constructor that takes a StandardKey (QKeySequence::Undo in this case.)

> Tools/MiniBrowser/qt/BrowserWindow.cpp:92
> +    redo->setShortcut(QKeySequence(Qt::CTRL | Qt::SHIFT | Qt::Key_Z));

Ditto (QKeySequence::Redo.)

[Chang Shu]

> > Source/WebKit2/UIProcess/API/qt/qwkpage.h:131
> > +    bool isModified() const;
> > +#ifndef QT_NO_UNDOSTACK
> > +    QUndoStack* undoStack() const;
> > +#endif
> 
> undoStack() should be replaced by d->undoStack inside QWKPage.
> Why do we need isModified() and the 'modified' property?

Right. We don't seem to need isModified(). It was in WebKit1, though.

[Chang Shu]

Created attachment 97592 [details]
fix patch 2

[Andreas Kling]

Comment on attachment 97592 [details]
fix patch 2

LGTM!

[WebKit Review Bot]

Comment on attachment 97592 [details]
fix patch 2

Clearing flags on attachment: 97592

Committed r89139: <http://trac.webkit.org/changeset/89139>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.