From kde-panel-devel  Thu Jul 26 10:57:52 2012
From: "Marco Martin" <notmart () gmail ! com>
Date: Thu, 26 Jul 2012 10:57:52 +0000
To: kde-panel-devel
Subject: Re: Review Request: KRunner Bookmarks: implement Chrome/Chromium support
Message-Id: <20120726105752.8892.43814 () vidsolbach ! de>
X-MARC-Message: https://marc.info/?l=kde-panel-devel&m=134330054924688
MIME-Version: 1
Content-Type: multipart/mixed; boundary="--===============8491380262329475172=="

--===============8491380262329475172==
Content-Type: multipart/alternative;
	boundary="===============2081046455915385296=="

--===============2081046455915385296==
Content-Type: text/plain; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable


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


what i can say quickly looking at it is that it looks cleaner and more expa=
ndible than the old one.
of course is a bit big, so will indeed need more testing, but most importan=
t thing for the first merge is that there aren't significant regressions co=
mpared to what there is already (konq, firefox, opera)

- Marco Martin


On July 26, 2012, 10:23 a.m., Marco Gulino wrote:
> =

> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/105673/
> -----------------------------------------------------------
> =

> (Updated July 26, 2012, 10:23 a.m.)
> =

> =

> Review request for Plasma.
> =

> =

> Description
> -------
> =

> I've nearly finished my work on the Bookmarks krunner, at least I think i=
t's ready for a review.
> Main focus: add support for chrome (and chromium).
> Side effects: refactoring, a few unit tests, and Favicons for firefox (it=
's very similar for chrome, they're stored as sqlite blob, so adding favico=
n for chrome meant making easy to add them to firefox too).
> I also removed some code seeming dead, or duplicated.
> =

> I'm mostly unhappy with the favicon fetch from firefox and chrome: i had =
to save them to temp file since it's impossible to load QPixmap in krunner =
plugins.
> =

> What's still missing: maybe a little more testing (both unit and manual) =
and cleanup.
> =

> branch: plasma/bookmarksrunner-chrome-gulino, path /plasma/generic/runner=
s/bookmarks
> =

> =

> Diffs
> -----
> =

>   plasma/generic/runners/bookmarks/.gitignore PRE-CREATION =

>   plasma/generic/runners/bookmarks/CMakeLists.txt 39d7834 =

>   plasma/generic/runners/bookmarks/bookmarkmatch.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/bookmarkmatch.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/bookmarksrunner.h d7545af =

>   plasma/generic/runners/bookmarks/bookmarksrunner.cpp aa3d45d =

>   plasma/generic/runners/bookmarks/bookmarksrunner_defs.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browser.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browserfactory.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browserfactory.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/chrome.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/chrome.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/chromefindprofile.h PRE-CREAT=
ION =

>   plasma/generic/runners/bookmarks/browsers/chromefindprofile.cpp PRE-CRE=
ATION =

>   plasma/generic/runners/bookmarks/browsers/findprofile.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/firefox.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/firefox.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/kdebrowser.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/kdebrowser.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/opera.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/browsers/opera.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/favicon.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/favicon.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/faviconfromblob.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/faviconfromblob.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/fetchsqlite.h PRE-CREATION =

>   plasma/generic/runners/bookmarks/fetchsqlite.cpp PRE-CREATION =

>   plasma/generic/runners/bookmarks/plasma-runner-bookmarks.desktop b75259=
1 =

>   plasma/generic/runners/bookmarks/tests/CMakeLists.txt PRE-CREATION =

>   plasma/generic/runners/bookmarks/tests/chrome-config-home/.config/chrom=
ium/Local PRE-CREATION =

>   plasma/generic/runners/bookmarks/tests/chrome-config-home/Chrome-Bookma=
rks-Sample.json PRE-CREATION =

>   plasma/generic/runners/bookmarks/tests/chrome-config-home/Chrome-Bookma=
rks-SecondProfile.json PRE-CREATION =

>   plasma/generic/runners/bookmarks/tests/testchromebookmarks.h PRE-CREATI=
ON =

>   plasma/generic/runners/bookmarks/tests/testchromebookmarks.cpp PRE-CREA=
TION =

> =

> Diff: http://git.reviewboard.kde.org/r/105673/diff/
> =

> =

> Testing
> -------
> =

> =

> Thanks,
> =

> Marco Gulino
> =

>


--===============2081046455915385296==
Content-Type: text/html; charset="utf-8"
MIME-Version: 1.0
Content-Transfer-Encoding: quoted-printable




<html>
 <body>
  <div style=3D"font-family: Verdana, Arial, Helvetica, Sans-Serif;">
   <table bgcolor=3D"#f9f3c9" width=3D"100%" cellpadding=3D"8" style=3D"bor=
der: 1px #c9c399 solid;">
    <tr>
     <td>
      This is an automatically generated e-mail. To reply, visit:
      <a href=3D"http://git.reviewboard.kde.org/r/105673/">http://git.revie=
wboard.kde.org/r/105673/</a>
     </td>
    </tr>
   </table>
   <br />





 <pre style=3D"white-space: pre-wrap; white-space: -moz-pre-wrap; white-spa=
ce: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">what i can=
 say quickly looking at it is that it looks cleaner and more expandible tha=
n the old one.
of course is a bit big, so will indeed need more testing, but most importan=
t thing for the first merge is that there aren&#39;t significant regression=
s compared to what there is already (konq, firefox, opera)</pre>
 <br />







<p>- Marco</p>


<br />
<p>On July 26th, 2012, 10:23 a.m., Marco Gulino wrote:</p>






<table bgcolor=3D"#fefadf" width=3D"100%" cellspacing=3D"0" cellpadding=3D"=
8" style=3D"background-image: url('http://git.reviewboard.kde.org/media/rb/=
images/review_request_box_top_bg.png'); background-position: left top; back=
ground-repeat: repeat-x; border: 1px black solid;">
 <tr>
  <td>

<div>Review request for Plasma.</div>
<div>By Marco Gulino.</div>


<p style=3D"color: grey;"><i>Updated July 26, 2012, 10:23 a.m.</i></p>






<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Descripti=
on </h1>
 <table width=3D"100%" bgcolor=3D"#ffffff" cellspacing=3D"0" cellpadding=3D=
"10" style=3D"border: 1px solid #b8b5a0">
 <tr>
  <td>
   <pre style=3D"margin: 0; padding: 0; white-space: pre-wrap; white-space:=
 -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap=
: break-word;">I&#39;ve nearly finished my work on the Bookmarks krunner, a=
t least I think it&#39;s ready for a review.
Main focus: add support for chrome (and chromium).
Side effects: refactoring, a few unit tests, and Favicons for firefox (it&#=
39;s very similar for chrome, they&#39;re stored as sqlite blob, so adding =
favicon for chrome meant making easy to add them to firefox too).
I also removed some code seeming dead, or duplicated.

I&#39;m mostly unhappy with the favicon fetch from firefox and chrome: i ha=
d to save them to temp file since it&#39;s impossible to load QPixmap in kr=
unner plugins.

What&#39;s still missing: maybe a little more testing (both unit and manual=
) and cleanup.

branch: plasma/bookmarksrunner-chrome-gulino, path /plasma/generic/runners/=
bookmarks
</pre>
  </td>
 </tr>
</table>





<h1 style=3D"color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b>=
 </h1>
<ul style=3D"margin-left: 3em; padding-left: 0;">

 <li>plasma/generic/runners/bookmarks/.gitignore <span style=3D"color: grey=
">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/CMakeLists.txt <span style=3D"color: =
grey">(39d7834)</span></li>

 <li>plasma/generic/runners/bookmarks/bookmarkmatch.h <span style=3D"color:=
 grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/bookmarkmatch.cpp <span style=3D"colo=
r: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/bookmarksrunner.h <span style=3D"colo=
r: grey">(d7545af)</span></li>

 <li>plasma/generic/runners/bookmarks/bookmarksrunner.cpp <span style=3D"co=
lor: grey">(aa3d45d)</span></li>

 <li>plasma/generic/runners/bookmarks/bookmarksrunner_defs.h <span style=3D=
"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browser.h <span style=3D"color: grey"=
>(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browserfactory.h <span style=3D"color=
: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browserfactory.cpp <span style=3D"col=
or: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/chrome.h <span style=3D"colo=
r: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/chrome.cpp <span style=3D"co=
lor: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/chromefindprofile.h <span st=
yle=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/chromefindprofile.cpp <span =
style=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/findprofile.h <span style=3D=
"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/firefox.h <span style=3D"col=
or: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/firefox.cpp <span style=3D"c=
olor: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/kdebrowser.h <span style=3D"=
color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/kdebrowser.cpp <span style=
=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/opera.h <span style=3D"color=
: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/browsers/opera.cpp <span style=3D"col=
or: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/favicon.h <span style=3D"color: grey"=
>(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/favicon.cpp <span style=3D"color: gre=
y">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/faviconfromblob.h <span style=3D"colo=
r: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/faviconfromblob.cpp <span style=3D"co=
lor: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/fetchsqlite.h <span style=3D"color: g=
rey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/fetchsqlite.cpp <span style=3D"color:=
 grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/plasma-runner-bookmarks.desktop <span=
 style=3D"color: grey">(b752591)</span></li>

 <li>plasma/generic/runners/bookmarks/tests/CMakeLists.txt <span style=3D"c=
olor: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/tests/chrome-config-home/.config/chro=
mium/Local <span style=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/tests/chrome-config-home/Chrome-Bookm=
arks-Sample.json <span style=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/tests/chrome-config-home/Chrome-Bookm=
arks-SecondProfile.json <span style=3D"color: grey">(PRE-CREATION)</span></=
li>

 <li>plasma/generic/runners/bookmarks/tests/testchromebookmarks.h <span sty=
le=3D"color: grey">(PRE-CREATION)</span></li>

 <li>plasma/generic/runners/bookmarks/tests/testchromebookmarks.cpp <span s=
tyle=3D"color: grey">(PRE-CREATION)</span></li>

</ul>

<p><a href=3D"http://git.reviewboard.kde.org/r/105673/diff/" style=3D"margi=
n-left: 3em;">View Diff</a></p>




  </td>
 </tr>
</table>








  </div>
 </body>
</html>


--===============2081046455915385296==--

--===============8491380262329475172==
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel

--===============8491380262329475172==--