[rekonq] Review Request 108902: Honor configuration for Web Shortcuts

Dimitrios Christidis dchristidis at ceid.upatras.gr
Mon Feb 11 08:59:34 UTC 2013


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

Review request for rekonq and Dimitrios Christidis.


Description
-------

The issue is partly described in bug #309470. Rekonq does not honor two configuration options for Web Shortcuts: "EnableWebShortcuts" and "UsePreferredWebShortcutsOnly".

The first part of the patch attempts to address this issue. I am not particularly proud of the current solution; please advise me on proper variable naming and overall code structure. Furthermore, I am not aware of the correct way to compare two KService::Ptr objects. I ended up comparing the filename of the desktop entry, but is there a better way? Is it even correct? Lastly, should SearchEngine::favorites() be modified as well? If not, preferred engine will still appear in the suggestions, but won't be usable.

The second part of the patch is about the change of "Search Engines" to "Web Shortcuts". Should the conditional compilation directives be removed or is it too early?


This review will be split into two commits.


This addresses bug 309470.
    /show_bug.cgi?id=309470


Diffs
-----

  src/settings/settingsdialog.cpp 11575f7 
  src/searchengine.cpp 742e9bc 

Diff: http://git.reviewboard.kde.org/r/108902/diff/


Testing
-------

+ With EnableWebShortcuts=false:
Typing a web shortcut followed by the delimiter does not suggest any engine. Pressing Enter uses the default engine.

+ With EnableWebShortcuts=true and UsePreferredWebShortcutsOnly=true:
Engines that are not preferred are not used.

+ With EnableWebShortcuts=true and UsePreferredWebShortcutsOnly=false:
Works as before.


Thanks,

Dimitrios Christidis

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/rekonq/attachments/20130211/02dd7f57/attachment.html>


More information about the rekonq mailing list