> On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/sessionmanager.cpp, line 91 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13941#file13941line91> > > > > is base64 really necessary if it's already in a CDATA section within a > > xml document ?
the QString(QByteArray) constructor reads out the qbytearray until the first byte with the value 0. But somehow every second byte in the history-bytearray is 0, so the only way I see to get the whole array is base64. > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/sessionmanager.cpp, line 186 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13941#file13941line186> > > > > I'm a bit confused here... Did you mean to call the slot (without emit) > > or emit the currentChanged signal ? the emit is definitely a mistake. Thanks for pointing out :) > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/sessionmanager.cpp, line 201 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13941#file13941line201> > > > > 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) ok. > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/sessionmanager.cpp, line 219 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13941#file13941line219> > > > > how about using something like this instead: > > foreach (const QDomElement element, document.elementsByTagName("tab")) elementsByTagName() returns a QDomNodeList which isn't a valid type for Q_FOREACH. > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/history/historymanager.h, line 129 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13936#file13936line129> > > > > 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. ok, I'm fine with that. > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/mainview.cpp, line 104 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13938#file13938line104> > > > > 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... I don't see a better way currently.. (I'll have a look at it again) > On April 7, 2011, 10:34 p.m., Pierre Rossi wrote: > > src/mainview.cpp, line 654 > > <http://git.reviewboard.kde.org/r/100604/diff/3/?file=13938#file13938line654> > > > > Is this really necessary ? (considering the takeFirst() above) *just recognizes that takeFirst() removes the item already* no it's not necessary. - Anton ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/100604/#review2480 ----------------------------------------------------------- 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
