Review Request: Activities support for Konqueror

[David Faure]

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



konqueror/src/konqview.h
<http://git.reviewboard.kde.org/r/106908/#comment16218>

    You should switch to #cmakedefine01 in the config-apps.h file, and use #if here.
    
    This has the additional benefit that if you forget to include the header file, or make a typo in the #if line, the compiler tells you nicely.
    
    We are switching all of KDE Frameworks to that, please do the same in new code.
    
    (Doesn't have to be done in this review request of course, since apparently it will affect existing code in the rest of the module, too...)



konqueror/src/konqview.cpp
<http://git.reviewboard.kde.org/r/106908/#comment16219>

    Is there a risk that this static pointer becomes dangling when this view is destroyed? I guess you're relying on FocusOut being received, but I wonder if this is 100% safe. Worst case an event filter could eat the FocusOut (but ok it would be a bug to have such an over-hungry eventfilter), or a Qt bug could make it fail to deliver the FocusOut before the view gets deleted. I'm not completely sure there's a bug here, but it just sounds risky to rely on an event to be received in order to avoid a crash.


- David Faure


On Oct. 17, 2012, 3:27 p.m., Ivan Čukić wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/106908/
> -----------------------------------------------------------
> 
> (Updated Oct. 17, 2012, 3:27 p.m.)
> 
> 
> Review request for KDE Base Apps and David Faure.
> 
> 
> Description
> -------
> 
> Konqueror reports the open/close document events to activity manager daemon.
> 
> By knowing which window contains which documents and which one is in focus, we can do the following:
> 
> - collect the statistics about visited pages. Further, this provides a score for each document visited, that depends on the number of times it was open, the time the user spent on that location, and the time passed since the last visit.
> - availability of a global/workspace applet that allows sharing the current document via e-mail, social networks; bookmarking and rating the link, or connecting it to the current activity. (advantage of this is a unified UI for sharing/rating/linking that works with any application)
> - jump-lists (not impl. yet in plasma) to list top rated documents on a launcher icon or in the task manager applet
> - krunner can sort the documents based on the score
> - more things that I haven't thought of yet
> 
> There is no need to *use* ativities to have these benefits. Activities just serve as manual data clustering to provide more useful scores compared to the one-activity approach.
> 
> 
> Diffs
> -----
> 
>   konqueror/src/CMakeLists.txt 529909d 
>   konqueror/src/konqview.h 45c5bde 
>   konqueror/src/konqview.cpp 2ee9896 
> 
> Diff: http://git.reviewboard.kde.org/r/106908/diff/
> 
> 
> Testing
> -------
> 
> The event reporting was watched in two ways:
> - by using the SLC applet (planned to be included in 4.10, currently plasma-mobile) - when konqueror opens a url, the icons light-up and when clicking on an icon, it show whether it reports the document properly
> - by using sqlite3 to browse the .kde/share/apps/activitymanager/resources/database - 'select * from nuao_DesktopEvent;'
> 
> Konqueror was used to open webpages and local directories; urls were opened in multiple windows, multiple tabs, or split-views, and both open/close and focusin/focusout events were registered correctly.
> 
> 
> Thanks,
> 
> Ivan Čukić
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20121017/8916330a/attachment.htm>