Review Request: Adds Firefox bookmark support to the bookmark runner

[Aaron Seigo]

> On 2009-11-06 00:37:53, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/bookmarksrunner.cpp, line 64
> > <http://reviewboard.kde.org/r/2085/diff/1/?file=13824#file13824line64>
> >
> >     what's the use case for the configurability here?
> 
>  wrote:
>     If the runner detects the wrong profile and because of that uses the wrong database the user could change the database file manually. I think it additionally prevents from scanning the profile.ini every time.

can profile.ini change? if so, then it would need to be checked every time anyways, no?


> On 2009-11-06 00:37:53, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/bookmarksrunner.cpp, line 66
> > <http://reviewboard.kde.org/r/2085/diff/1/?file=13824#file13824line66>
> >
> >     KSimpleConfig would be faster (skips merging, globals, etc)
> 
>  wrote:
>     KSimpleConfig doesn't exist anymore. I used KConfig("file", KConfig::SimpleConfig) as mentioned in ksimpleconfig.h

oops; still have kde3 on the brain sometimes ;)


> On 2009-11-06 00:37:53, Aaron Seigo wrote:
> > trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/bookmarksrunner.cpp, line 259
> > <http://reviewboard.kde.org/r/2085/diff/1/?file=13824#file13824line259>
> >
> >     since we have the power of SQL at our fingertips here, what about using the WHERE clause by adding, e.g. query("SELECT fk,title FROM moz_bookmarks WHERE type = 1 AND title LIKE  '" + term.replace("'", "\\'") + "'")
> 
>  wrote:
>     I did the point about the SQL query you mentioned below so I didn't do this one, because it didn't work with this.

does this work: 
const QString escapedTerm = term.replace("'", "\\'");
QSqlQuery query("SELECT moz_bookmarks.fk, moz_bookmarks.title, moz_places.url, moz_places.favicon_id FROM moz_bookmarks, moz_places WHERE moz_bookmarks.type = 1 AND moz_bookmarks.fk = moz_places.id AND (moz_bookmarks.title LIKE  '" + escapedTerm + "' or moz_places.url LIKE '" + escapedTerm + "')";


- Aaron


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/2085/#review2947
-----------------------------------------------------------


On 2009-11-06 18:17:32, Jan Gerrit Marker wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2085/
> -----------------------------------------------------------
> 
> (Updated 2009-11-06 18:17:32)
> 
> 
> Review request for Plasma.
> 
> 
> Summary
> -------
> 
> This patch adds Firefox support to the bookmark runner. The database, where the bookmarks are saved in, is copied and then used for getting the bookmarks. The support was added a way that you can add other browsers easily (e.g. by adding a method "whichBrowser()", an enumeration "Browser" and own methods for each browser (Firefox and Konqueror at the moment)). The path to the original database file is stored in the runners configuration group. The browser to get the bookmarks from is taken from the command set in the applications systemsettings module.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/CMakeLists.txt 1045755 
>   trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/bookmarksrunner.h 1045755 
>   trunk/KDE/kdebase/workspace/plasma/generic/runners/bookmarks/bookmarksrunner.cpp 1045755 
> 
> Diff: http://reviewboard.kde.org/r/2085/diff
> 
> 
> Testing
> -------
> 
> I tested it and it was fine with Firefox 3.5.4 and Konqueror from trunk.
> 
> 
> Thanks,
> 
> Jan Gerrit
> 
>