Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 339340

Summary: Search history "Open in new" only allows you to open one additional Search view
Product: [Eclipse Project] Platform Reporter: Stewart Francis <stew>
Component: SearchAssignee: Dani Megert <daniel_megert>
Status: RESOLVED FIXED QA Contact:
Severity: minor    
Priority: P3 CC: daniel_megert, markus.kell.r
Version: 3.6.1   
Target Milestone: 3.8 M5   
Hardware: All   
OS: All   
Whiteboard:
Attachments:
Description Flags
org.eclipse.search patch none

Description Stewart Francis CLA 2011-03-09 07:17:56 EST
Build Identifier: M20100909-0800

From the SearchHistorySelectionDialog selecting a search result and clicking the "Open in new" button should open the selected search result in a new instance of SearchView.  Infact this only works for one additional search view due (I believe) to a bug in the implementation of ShowSearchHistoryDialog.

If an "Open in new" is requested, the ShowSearchHistoryDialog pins the search view that was used to display the dialog, and requests the InternalSearchUI show the historical search result.  The pinning forces InternalSearchUI to open a new SearchView.  However, only the SearchView used to invoke the dialog is pinned.  Any additional SearchViews are not pinned and as such are reused (undesirably!)

It is possible to force a new SearchView to open by manually pinning all of the active SearchViews before trying to open an additional one.

Reproducible: Always

Steps to Reproduce:
1. Run 3 different searches
2. In the search dialog, display the SearchhistorySelectionDialog by invoking the History... option from the search history menu
3. Select the second oldest search result, and clik "Open in new".  Observe the search result is displayed in a new SearchView
4. Repeat 2 + 3, selecting the oldest search result.  Observe how one of your search result views will be reused, regardless of the "Open in new" request.
Comment 1 Markus Keller CLA 2011-03-09 13:05:24 EST
I agree, that sounds wrong. If you could write a patch to fix this, that would be great :-)
Comment 2 Stewart Francis CLA 2011-03-10 19:16:18 EST
I've got a working patch, which I'll submit as soon as I have legal approval from my employer.
Comment 3 Stewart Francis CLA 2011-05-26 19:57:35 EDT
Created attachment 196713 [details]
org.eclipse.search patch

Finally got approval, whew!

I had to add a new method SearchViewManager.activateNewSearchView() in order to preserve the current behaviour of activateSearchView(true) which activates a search view in LRU order avoiding pinned views.  The new method just forces the generation of a new secondaryId, instead of potentially reusing one.

The rest of the changes are to invoke the new API and reorganising for code reuse.
Comment 4 Dani Megert CLA 2011-06-08 04:49:49 EDT
Hi Stewart, I did not yet review the actual code in the patch but a first glance shows that the copyright header isn't updated. Please add your credentials, e.g.
Dani Megert <dani@eclipse.org> - this is a bug - https://bugs.eclipse.org...
Comment 5 Stewart Francis CLA 2011-07-07 20:14:53 EDT
Hi Dani, I'm an IBMer, so I guess any copyright issues will be covered by the existing copyright statements.  If you need me to make any additions, let me know.
Comment 6 Dani Megert CLA 2011-12-15 10:55:47 EST
Thanks for the patch Stewart. I decided to fix it in a less intrusive was i.e. with less changes. Please try it out in one of the upcoming builds.

Fixed in master: c949d80f0dbcd3579db24de3d55babb130fb984c