----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100604/#review2480 -----------------------------------------------------------
Thumbs up to restoring the history along with the tab ! :) src/history/historymanager.h <http://git.reviewboard.kde.org/r/100604/#comment2149> It feels "url" would work well for this field's name, and a bunch of lines above would get to stay the same. Otherwise title should be called currentTitle for consistency, but since it's quite obvious I think you can drop the "current" altogether. src/mainview.cpp <http://git.reviewboard.kde.org/r/100604/#comment2153> do we really need this all foreach loop ? It doesn't seem very likely to me that two closed tabs will have the same history... src/mainview.cpp <http://git.reviewboard.kde.org/r/100604/#comment2151> Is this really necessary ? (considering the takeFirst() above) src/sessionmanager.cpp <http://git.reviewboard.kde.org/r/100604/#comment2150> is base64 really necessary if it's already in a CDATA section within a xml document ? src/sessionmanager.cpp <http://git.reviewboard.kde.org/r/100604/#comment2148> I'm a bit confused here... Did you mean to call the slot (without emit) or emit the currentChanged signal ? src/sessionmanager.cpp <http://git.reviewboard.kde.org/r/100604/#comment2146> Nice catch on this refactoring, but I feel it could go in a separate micro-commit, since it is not specially tied to this change. (r=me for that commit if you feel like doing so) src/sessionmanager.cpp <http://git.reviewboard.kde.org/r/100604/#comment2147> how about using something like this instead: foreach (const QDomElement element, document.elementsByTagName("tab")) - Pierre On April 7, 2011, 6:48 p.m., Anton Kreuzkamp wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/100604/ > ----------------------------------------------------------- > > (Updated April 7, 2011, 6:48 p.m.) > > > Review request for rekonq. > > > Summary > ------- > > Restore the history of the tab when restoring a tab from the list of closed > tabs, by open last closed tab or by restoring the session. > Therefore I ported the session-file to xml, as it allows more complex > session-files (which is also needed for multiple sessions support). > (Doesn't require a qtwebkit-patch anymore) > > > Diffs > ----- > > src/application.cpp 1fe936f > src/history/historymanager.h eb8d78d > src/mainview.h acc2d8c > src/mainview.cpp b34acc3 > src/newtabpage.cpp a598066 > src/sessionmanager.h 2297290 > src/sessionmanager.cpp 68bc635 > src/tabbar.cpp 1ab357f > > Diff: http://git.reviewboard.kde.org/r/100604/diff > > > Testing > ------- > > tested and everything seems to work fine. > > > Thanks, > > Anton > >
_______________________________________________ rekonq mailing list [email protected] https://mail.kde.org/mailman/listinfo/rekonq
