Summary: | WorkbenchPage: ViewReference handling of duplicate Parts seems wrong | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Benedikt Kuntz <benedikt.kuntz> |
Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> |
Status: | NEW --- | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | benedikt.kuntz, Lars.Vogel, mistria |
Version: | 4.16 | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | Windows 10 | ||
See Also: | https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168308 | ||
Whiteboard: |
Description
Benedikt Kuntz
2020-08-27 05:03:52 EDT
Sounds like ViewReferences needs to check for the ID instead. Is this also your assessment, Benedikt? New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/168308 (In reply to Lars Vogel from comment #1) > Sounds like ViewReferences needs to check for the ID instead. Is this also > your assessment, Benedikt? Well I am not too much into that Workbench handling, but I guess there should be one ViewReference for each MPart instance. If that's the case, then the contains-check in WorkbenchPage should also check for equality instead of the ID (like the getViewReference method). Otherwise there will always be only one ViewReference per ID. I added a gerrit to show what I *think* is more correct... But I cannot really see what kind of other implications this could have. Lars, how should we proceed here? I am not really confident that we should merge the gerrit, it seems to me that this could have some unwanted implications. But on a pure logical basis, the fix seems correct to me... (In reply to Benedikt Kuntz from comment #4) > Lars, how should we proceed here? I am not really confident that we should > merge the gerrit, it seems to me that this could have some unwanted > implications. But on a pure logical basis, the fix seems correct to me... Lets review for 4.18M1, 4.17 is in release freeze. Mass change to 4.19 M1, please update the target if you have other plans. Mass move 4.19 M1 bugs to M3 Removing milestone from 4.19 M3 to 4.19, please re-target accordingly. Mass move out of 4.19 Do you think the root cause may be that the elementId is incorrect? I'm wondering whther this might be a deeper issue. Not sure what you mean with incorrect elementId? The id (in my case) comes from the e4 model (i have a part descriptor with id "xyz") that is added to a part stack one or more times. I did not spot any other problems with the ViewReference handling except for the one mentioned before with the NPE in the DefaultHelpUI... If I understand correctly, the problem is that 2 distinct view reference do share the same elementId, isn't it? If so, wouldn't it be instead expected that those have different elementId? Yes, I assume that is the problem. It is expected in the contains method of the class, but not in the getViewReference method. I cannot say if they have to be different from the e4 point of view, but actually I guess not because I have never heard that you have to change the element ID manually during runtime... Mass Move out of 4.20 |