GCI Task: Context menu improvements

Review Request #100167 - Created Nov. 24, 2010 and discarded

Information
Nikhil Marathe
rekonq
Reviewers
rekonq
This was a Google Code In 2010 task. Context menu improvements are by Furkan Uzumcu <furkanuzumcu@gmail.com>. They include:

* Find... and Print... actions in context menu
* Detect URL like selected text and offer to treat it as a URL and open in new window/tab
* View Image in new Tab
Tested and works OK.
Panagiotis Papadopoulos

   
src/webview.cpp (Diff revision 1)
 
 
View &Image in New Tab

-> remove the “a”
Andrea Diamantini
I can confirm it works well. But I'm not really satisfied with this change. In fact I remember we debated a lot about contextual actions. First 2 considerations:
1) the find action is shown also when pointing an image. IMHO, in that case it has to not be shown.
2) with the "view image in a new tab" we reached the sixth action image related!! IMHO it is a bit too much :)
I have a suggestion about: why don't you change the "actual" view image action to show it in a new tab as default?
  1. I would suggest splitting up this review request into three parts:
    
    * Find... and Print... actions in context menu
    * Detect URL like selected text and offer to treat it as a URL and open in new window/tab
    * View Image in new Tab
    
    ;-)
    
    that way we can more easily discuss each review separately…
Benjamin Poulain

   
src/webview.h (Diff revision 1)
 
 
 
 
Those comments should not be here. They could be in the cpp file, before the function, but not here after the prototype.
src/webview.cpp (Diff revision 1)
 
 
 
I would prefer a new temp variable:
KAction *const findAction = ...
menu.addAction(findAction);
src/webview.cpp (Diff revision 1)
 
 
 
 
This is invalid IMHO. I would build a KUrl with the text and use KUrl::isValid() to assert of the validity of the Url.
src/webview.cpp (Diff revision 1)
 
 
The comment should be on the line before, not after the code.

The comment should end with a dot since it is a complete sentence (like on WebKit ... :)).
src/webview.cpp (Diff revision 1)
 
 
 
Please give a better name to the variable "a".
src/webview.cpp (Diff revision 1)
 
 
 
 
 
Invalid indentation.
src/webview.cpp (Diff revision 1)
 
 
 
Please give a better name to the variable "a".
Andrea Diamantini
Nikhil, Furkan is directly providing his patch. Please close this.
Loading...