Closed Bug 15176 Opened 22 years ago Closed 19 years ago

[FEATURE] Search context menu item for selected text

Categories

(SeaMonkey :: UI Design, enhancement, P2)

enhancement

Tracking

(Not tracked)

VERIFIED FIXED
mozilla0.9.8

People

(Reporter: bugs, Assigned: samir_bugzilla)

References

Details

(Keywords: perf, Whiteboard: [ready to checkin])

Attachments

(6 files)

[ note to self so I don't forget this. ]

Possibly: add context menu items to navigator that allows user to:

a) open selection as a query in the smartsearch window (e.g. select "cats and
dogs" on a webpage, context-click, and select "Search for [cats and dogs...]"

b) open the selection as a link (it is common for people to insert the URL of a
webpage, but neglect to make it an HTML link, and thus subject the viewer to the
task of copy-pasting it into the location bar. A smarter web browser would allow
user to do this with less effort)
Status: NEW → ASSIGNED
Target Milestone: M15
Re-evaluate after beta 1.
*** Bug 25799 has been marked as a duplicate of this bug. ***
Ideally, this would be extensible. I have bookmarklets in 4.5 set up to do what 
this bug is describing (using document.getSelection()), for
* dictionary definition
* Google search
* Bugzilla bug number.

However this is just my personal set of selection processors, and if this is 
reimplemented using a context menu, I should be able to add/remove others from 
that menu.

You also run the risk of the link context menu getting very crowded. It's got 13 
items in it already, and about 7 of those are what I would call essential. 
Adding an arbitrary number of other items to this menu could make it quite ugly.
Since I was the one who submitted 25799, I'd like to request at least b) to be
implemented. Being able to do other things with selections sounds fantastic as
well. Ugly menus would be a small price to pay ;-).
not a priority, pushing out as far as possible.
Target Milestone: M15 → M20
Move to "Future" milestone.
Target Milestone: M20 → Future
Now that Mozilla has scrolling menus, this wouldn't look as ugly.

A good way to make this customizable would be to make a special folder in 
bookmarks.  When you right-click a selection, the context menu would include 
each of the items on that menu.
*** Bug 60615 has been marked as a duplicate of this bug. ***
Copying more useful summary from dup bug 60615.
Summary: [FEATURE] Intelligent Selection handling in browser → [FEATURE] Enhanced context menu for selected text
Another thing that would be useful on the context menu is view partial source 
(bug 49721).
*** Bug 17751 has been marked as a duplicate of this bug. ***
nav triage team:

Don't think we'll get to this for beta1, too many other things to fix, marking 
nsbeta1-
Keywords: nsbeta1-
QA Contact: don → sairuh
*** Bug 64454 has been marked as a duplicate of this bug. ***
reassigning to me because ben hasn't shown much interest.
Assignee: ben → alecf
Status: ASSIGNED → NEW
Adding myself and Matt to cc-list.
Please change OS and platform to ALL.
OS: Windows 95 → All
Hardware: PC → All
OK, bugzilla let me do it myself :-) Apologies for spam.

From german:
This does google search in 4.x .   For selected.
We would like it also for any word clicked on.

javascript:docSel=document.getSelection();if(!docSel){void(docSel=prompt('Enter
a search term\n\nTip: you may also use this with selected Text in a
Browser window.',''))};if(docSel){document.location='
http://www.google.com/search?q='+escape(docSel);} <
javascript:docSel=document.getSelection();if(!docSel){void(docSel=prompt(&apos;E
nter
a search term\n\nTip: you may also use this with selected Text in a
Browser
window.&apos;,&apos;&apos;))};if(docSel){document.location=&apos;
http://www.google.com/search?q=&apos;+escape(docSel);}>
Blocks: 10080
But can we have a function to search for user entered text within the source ?
For me that would be by far the most useful option.
*** Bug 78182 has been marked as a duplicate of this bug. ***
Sorry I just realised my last comment was attached to the wrong bug. The 
feature I really wanted here was to highlight text and jump to it as a link. 
Would it be possible to see if selected text starts with something 
like 'http://' or 'ftp://' and then add the option 'jump to as link' to the 
context menu ?
Is there really a need for that enhancement and why?
like any feature, because someone thought it was a good idea. bugzilla is not
the forum to justify bugs, so let's keep this discussion on how we're going to
actually implement this feature.
Well, otherwise how else do you jump to a link in a page, if that link is not an
anchor - answer: double click in the location bar, right click, delete; select
text, right click, copy, click in location bar, right click, paste, press enter.

10 operations when it could be reduced to 3 (select text, right click, jump to
link).

Note that this was suggestion b) of the original poster of this enhancement.
It's already possible to jump to a URL in 5 steps: select url, copy, Ctrl+L 
(Moz) or Alt+D (IE), paste, enter.  That doesn't necessarily mean it's not 
worth it to include a "jump to url" option on the selection context menu, 
though.
This bug has been in the system since Sept 1999... Someone could iterate from 
the comments of "matt@netscape.com 2001-03-12 22:22", and use newURI()
http://lxr.mozilla.org/seamonkey/source/netwerk/base/public/nsIIOService.idl

If newURI() succeeds on the selection, that would mean the selected sting
is a valid URI, and from there a "Go to..."  could be added to the context menu.

Regexp for other fancy things like "Mail to..." could be attempted if 
newURI() fails.

And the JS code for the context menu:
http://lxr.mozilla.org/seamonkey/search?string=contextmenu
A newcomer could start by following bug 32358 "changes to context menu for 
mailtos".
From bug 9274: Another option that would be nice to have on the selection
context menu is "Open Links in New Windows" or "Open Selected Links".
*** Bug 88518 has been marked as a duplicate of this bug. ***
reassign to default owner
Assignee: alecf → pchen
Target Milestone: Future → ---
No longer blocks: 75338
arrgh! i keep entering stuff in the wrong field.<apologies/>
No longer blocks: 75338
Depends on: 75338
=> matt
Assignee: pchen → matt
Attached patch patch for searchSplinter Review
Above is a patch for search.
Make regex or URI to find urls and dispatch that page
leave all others the rest as search.
Patch above supports search.  Passing teh selected text to openSearch in 
navigator.js to call the search function.

1. Image is not included because there is not an easy way at the moment to 
create a dynamic image attached to a menuitem.

2.This if one highlights a text and it is a URL then it should update the Open 
in new Window...
I believe this would make more sense to the user. If this was the case then we 
would have to add code to the Open in new Window menu command.  This would be 
done in the 

setTarget : function command.

We either should open a new bug for the functionality or make a new bug for the 
patch i created.


Morse, since your the context menu owner I think i need a review from you.
thanks.
+    isTextSelected : function() {
+       var result = false;
+       if (_content.getSelection() != "") {
+          var searchSelect = document.getElementById('context-searchselect'); 
+             searchSelect.setAttribute("label", "Search for \'" + ...
+          var result = true;
+       }
+       return result;
+    },

1. Remove the "var" on the second "var result", otherwise you will have two 
different result variables and you will be assigning "true" to the wrong one.

2. (nit) fix the indentation on the "searchSelect" line

With those two changes, r=morse
There's some localization issues with that code.  The label string "Search for
%s" should go in a .dtd/.properties file and the text placed into it, no?

Also, won't the context menu really suck if the text is at all long?  I think it
should be truncated at some reasonable length and an ellipsis appended.  E.g.,
    Search for 'some really big selection...'
Attached patch updated patchSplinter Review
1. extra var gone
2. no tabs....fixed editor to stop making them when i dont' want them.
3. Localized in props file.  What was I thinking.
4. The popup takes care of the fact that the string is to long it will cut it
off at something like 20 or so chars.  But none the less I added code to only
allow 15 chars and then if it has more i add "..." .  I didn't localize ... but
I can if someone thinks I should.
Thanks.
Comment on attachment 53206 [details] [diff] [review]
updated patch

r=law

I didn't realize that the popup would truncate automatically, but I think the ellipsis is a nice touch anyway and I don't think it needs localization (I hope not, I think I've hardcoded some of those myself).
Attachment #53206 - Flags: review+
Glad to see that you got rid of the unused label on context-searchselect
r=morse
sr=hyatt
This going to get checked in now that the tree is open?
fixed
Status: NEW → RESOLVED
Closed: 19 years ago
Resolution: --- → FIXED
No longer blocks: 10080
Since this bug became restricted to adding "search" to the selection context 
menu, I've opened bug 105580 for continued discussion.
Blocks: 102472
Summary: [FEATURE] Enhanced context menu for selected text → [FEATURE] Search context menu item for selected text
vrfy'd fixed using 2001.11.08.0x-comm bits on linux rh6.2, winnt and mac os
10.1. even nicely strips prepended whitespace and compresses multiple internal
whitespaces in the selection. :)

however, i noticed that this feature is missing in context menus for frames
--see bug 109171.

marlon, here's "another additional context menu item" to consider wrt the spec. :)
Status: RESOLVED → VERIFIED
Reopening to complete feature to spec.  For reference see: 
<http://www.mozilla.org/projects/ui/communicator/browser/search/>
Status: VERIFIED → REOPENED
Resolution: FIXED → ---
morse, please r.
alecf, please sr.
Status: REOPENED → ASSIGNED
Priority: P3 → P2
Target Milestone: --- → mozilla0.9.7
Could you please post a diff with -w so I can see the real changes and not be 
lost in all the whitespace changes.  Thanks.
Comment on attachment 60697 [details] [diff] [review]
Add icon and engine name to search context menu item to match spec.

very cool, but isn't this going to be slow, to retrieve the icon every time the
menu pops up? seems like you can cache the value the first time the menu pops
up, and then watch for the pref to change.
->samir

adding perf kw in case implementation of this does result in slowness...
Assignee: matt → sgehani
Status: ASSIGNED → NEW
Keywords: perf
Steve: sure, but then be aware that my tabbing will look like it's off.  It
actually isn't because I'm fixing the 3 char tab in these search-related
functions to align with the file-style 4 char tab.  Funny you ask cause I almost
attached my diff -w.  Anyways, will comply with the request next revision.

Alec: will look into your suggestion.

Next patch will also include the fix for the feature missing in context menus in
frames which is sitting in my tree (bug 109171).  
Alec,
The gContextMenu gets destroyed during the "onpopuphiding" event.  So I can
create my own parallel global, gDefaultEngine, which is an nsIObserver and is
filled in the first time text is selected and we have to show the menu item. 
Only, now the gDefaultEngine will last for the life of the browser session
(i.e., till mozilla is killed) and so adds to bloat.  The bloat is tiny but
would you prefer this to getting the icon each time (my first implementation). 
I run on a 1.5GHz machine so it is hard for me to tell if the context menu is in
fact slowed down (my suspicion is that it isn't since the datasource is in
memory).  I would lean towards the first implementation but have the second
almost ready to go.
Status: NEW → ASSIGNED
morse/alecf, please r/sr latest patch.  Thanks.
Comment on attachment 60743 [details] [diff] [review]
Patch with Alec's pref-change observer approach (also diff -w per Steve's request).

r=morse
Attachment #60743 - Flags: review+
Attaching context menu with "funny" content. Is this how it's supposed to be?
The text will also contain the alt-text of an image if one exist.
Build id 2001120603.
Rune,
I can't reproduce your bug with a current build on Win2K.  Please open a new bug
report and include the URL and your system info.  Let's continue the
investigation in the new bug report.  Thanks. 
Samir,

Fair enough. I filed bug 114615.
Comment on attachment 60743 [details] [diff] [review]
Patch with Alec's pref-change observer approach (also diff -w per Steve's request).

sr=dveditz

A comment block delimiting the nsDefaultEngine class would make things clearer.
At first I didn't realize I'd wandered into a constructor and thought "this"
still referred to the context menu.
Attachment #60743 - Flags: superreview+
Whiteboard: [ready to checkin]
Target Milestone: mozilla0.9.7 → mozilla0.9.8
Note that this menuitem already visibly slows down the opening of the context
menu when you have all text selected (without the icon addition).
Keywords: patch
Checked in.
Status: ASSIGNED → RESOLVED
Closed: 19 years ago19 years ago
Resolution: --- → FIXED
this feature has been checked in (some time ago :).
Status: RESOLVED → VERIFIED
Product: Core → Mozilla Application Suite
You need to log in before you can comment on or make changes to this bug.