| Summary: | Secondary Problems view doesn't have a view menu -> not configurable | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Markus Keller <markus.kell.r> | ||||
| Component: | UI | Assignee: | Eric Moffatt <emoffatt> | ||||
| Status: | VERIFIED FIXED | QA Contact: | |||||
| Severity: | major | ||||||
| Priority: | P3 | CC: | albert.choi, ben.cox, bhobbs, bsd, daniel_megert, emoffatt, jpetrakis, kuebler, pwebster, rajeshwarcsit, sw, tsichevski | ||||
| Version: | 4.3 | ||||||
| Target Milestone: | 4.3 M7 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows 7 | ||||||
| Whiteboard: | |||||||
| Bug Depends on: | |||||||
| Bug Blocks: | 418689 | ||||||
| Attachments: |
|
||||||
|
Description
Markus Keller
You're almost certainly correct Markus (again...;-). What is the format for the viewId / secondaryId when you create the new view(s) ? > What is the format for the viewId / secondaryId when you create the new
> view(s) ?
I don't know, I'm just a user ;-). But when I set a breakpoint in org.eclipse.ui.internal.WorkbenchPage.showView(String, String, int), then I see:
viewID= "org.eclipse.ui.views.ProblemView"
secondaryID= "1"
However, the format doesn't seem to be an issue, since opening a new Search view works fine ("Show Previous Searches" toolbar button, and then use "Open in New"):
viewID= "org.eclipse.search.ui.views.SearchView"
secondaryID= "4"
The problem seems to be that the call to findPart(..) in org.eclipse.ui.internal.WorkbenchPage.busyShowView(String, int) finds an MPart it shouldn't find.
http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=f5392cb888caa6b0e52155bca9bb6fd01c29c01c This is Part 1 of the fix...the 'findReferences' wasn't splitting off the 'descriptorId' from the MPart's id, causing the handler's 'newSecondaryID' method to continually return the same secondary id... http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=9611c6e4bcdd5190d13a5b506402d002a5d890b3 This fixes the missing menu (and should also fix TB's). The issue is that we need *only* the primary ID when creating the menus / tb model elements since their ids get matched to the contributed menu using "menu:viewId" but here 'viewId' means only the primary id. NOTE: Still need to check where the default context menu is created... (In reply to comment #3) This commit completely broke org.eclipse.ui.IWorkbenchPage#findView(String): It usually just returns the "first" view now. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=e434b72fa7876b32d57dbd0e9367cc25faabcdf0 Oops, bad day at the office...thanks Markus. (In reply to comment #5) That fix solved the severe bug for regular views, but it was still wrong for secondary views. Fixed with http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=d89de46e7969b6bf3d920ff3f150f1fde95237cd http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=3e0f6334cf3c634b16d51d29478dca616ffd10ba NOTE: I just discovered a flaw in this one as well (likely tied to my changing the WorbenchPartReference's 'getId' to be 'correct'...amendment coming ! Markus, thanks for your work on this... While looking into it yesterday I realized that we're actually writing the code backwards. In e4 we store the 'compound id' (viewId:SecId) in the part so reversing the logic cleans up the code a lot (take a look). This commit also fixes the WorkbenchPartReference impl to (correctly) only return the descriptor id of the view. http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=bc9eda6cbcd650e67c86f50063b3d0a272555200 OK, this should be it I think. The issue was that the WPage's 'contains' method (used to prevent refs being added more than once) was using the new 'getId' code from the references to do the compare. This fixes it to use the embedded MPart's element id (which is == to what the old implementation of 'getId' did. Yes, looks good, thanks. Verified with multiple instances of the Problems and Search views. *** Bug 405563 has been marked as a duplicate of this bug. *** *** Bug 404795 has been marked as a duplicate of this bug. *** *** Bug 383309 has been marked as a duplicate of this bug. *** I'd sure like to see this backported or patched in 4.2.2 Verified in 4.3.0.I20130430-0031. Created attachment 230452 [details]
Patch to handle secondaryIDs for findViewReference
WorkbenchPage#findViewReference(String viewId, String secondaryId)
creates a composite id and delegates to findViewReference(String viewId) that does not look for secondary ids in tags. Therefore views with secondary ids can not be found.
The given patch (against a local svn repository) adds functionality to honor secondary ids.
Jens, please open a new bug with your patch. PW Jens, before opening a new defect please check whether the problem still exists, You may have been looking at some of the earlier code where the secondary Ids were stored separately from the element's 'id'...here's how it works now: 1) All views have an 'id', usually identical to that of the MPartDescriptor they're for 2) Views that are multi-instance may have a 'secondaryId' in order to distinguish them. While implementing the support for these we determined that having these ids split across different fields in the model was making things much harder to understand. The solution was to add code to handle 'compound' ids (ones that have a ':' in them) with the following rules: 1) When creating a new view we only use the substring before the ':' to find the part descriptor for the part. 2) When trying to place the view into a stack we have explicit code looking for the id '<viewId>:*", where 'viewId' is again the part before the colon. Other than these few rules there is no difference between single and multi-instance views which is why the current find references code looks the way it does. To me it seems much cleaner this way all around (i.e. when you look at a part's id in the debugger or whatever you can see the complete id...). I guess I missed that one http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?h=R4_2_maintenance&id=645c6b57845a594c7f0dc0c04a0b98ef88638464 Sorry for that one. Thanks for the explanation with secondary ids. *** Bug 409977 has been marked as a duplicate of this bug. *** FYI: The view menu is still missing in the secondary Problems view when a 4.2 workspace is used with 4.3, see bug 412013. (In reply to John P. Petrakis from comment #14) > I'd sure like to see this backported or patched in 4.2.2 Hi Marcus, Eric and All, We have recently migrated our code from eclipse 3.6.2 to 4.2.2 version. Everything seems working fine except few views (with secondary Id's). I have created a forum topic for this issue, Please find the details here. http://www.eclipse.org/forums/index.php/t/521940/ http://www.eclipse.org/forums/index.php/m/1109927/#msg_1109927 We are using secondary view ID's for these views. I have tried them in eclipse 4.3 and noticed them working fine. Toolbars are not missing in view's in 3.6.2 and 4.3 versions, but only in 4.2.2. I have seen this defect description(especially comment #4) and feel that this is the issue causing our problem. If this is the right fix and solves our problem, is it back ported to eclipse 4.2.2 as mentioned in comment #14 of this defect. if not when it will be backported? Our product supports only eclipse 4.2.2, so we need a work-around or quick fix for this issue in 4.2.2 itself. As this problems is not re-producible in 4.3, one of the fix must have solved the issue we are facing currently, So please do let me know how we can resolve this in 4.2.2. As we need it urgent, could you please respond to this quickly. Thanking you, Regards, Rajeshwar kalakuntla IBM (In reply to Rajeshwar Kalakuntla from comment #22) > If this is the right fix and solves our problem, is it back ported to > eclipse 4.2.2 as mentioned in comment #14 of this defect. No. > if not when it will be backported? 4.2.2 was the last release. You can request a fix via official process in your company. Ping me, if you don't know how that works. *** Bug 418084 has been marked as a duplicate of this bug. *** *** Bug 424389 has been marked as a duplicate of this bug. *** |