Bug 562472 - Changed behavior for placeholder and part IDs
Summary: Changed behavior for placeholder and part IDs
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.15   Edit
Hardware: PC Windows 10
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 562497
  Show dependency tree
 
Reported: 2020-04-24 11:23 EDT by Gerhard Kreuzer CLA
Modified: 2021-05-14 08:15 EDT (History)
4 users (show)

See Also:


Attachments
Sources of a minimal RCP app to reproduce the described effect (27.35 KB, application/x-zip-compressed)
2020-04-24 11:23 EDT, Gerhard Kreuzer CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gerhard Kreuzer CLA 2020-04-24 11:23:35 EDT
Created attachment 282555 [details]
Sources of a minimal RCP app to reproduce the described effect

When porting a RCP app from Eclipse V4.7 (Oxygen) to V4.15 (2020-03), I discovered strange behavior, that I "fixed" in my app by providing different IDs for placeholders and their referenced parts. Because I used the same IDs due to some advice I read somewhere, I asked the community, if someone can explain it to me (https://www.eclipse.org/forums/index.php/m/1823368/#msg_1823368)? [In my text I spoke from Eclipse Photon, which is wrong, I ment Eclipse Oxygen!] Ralf Theunissen asked for a minimal example to reproduce it, which I supply as attachment.
The example consists of three plug-ins (demo, demo.perspective and demo.fragment). The demo plug-in has two product definition files, one for the framework V4.7 and one for V4.15. 
If imported into and started from Eclipse IDE V4.7, the RCP app shows up with a sash control that has two parts in its left part stack (defined as placeholders in demo.fragment/fragment.e4xmi). 
If imported into and started from Eclipse IDE V4.15, the RCP app shows up with its sash, but the part stack is empty!?
As soon as you append something to the IDs of the placeholders (e.g. '.placeholder'), the app starts as expected with the two placeholders/parts visible.
(Please mind to employ the appropriate product definition!)
Is this a bug or just new behavior? I personally tend to see it as bug.
Comment 1 Rolf Theunissen CLA 2020-04-25 04:58:58 EDT
This a regression of the fixes for Bug 509644.

As result of that change, all elements in all contributed fragments must have a unique ID, otherwise they are not processed correctly. That is, some elements will be silently ignored.

AFAIK uniqueness of identifiers is not an requirement for the E4 model. If this is a requirement it should be validated and errors should be logged if this is violated.

Workaround is indeed to have an unique ID for each element.
Comment 2 Lars Vogel CLA 2020-04-25 05:11:47 EDT
Some IDs are not unique by design, e.g. the top level menu of every window should have the same id
Comment 3 Gerhard Kreuzer CLA 2020-04-25 08:50:29 EDT
I discovered the "solution" by playing around and did not try to understand the behavior by studying the source code. So when recognizing the pattern, that referenced elements and their placeholders should carry different IDs, I applied this scheme to all placeholder element pairs in my app, including an Area and its placeholder. But in the case of the Area providing different IDs did not work, but messed up the app's layout...

Furthermore (Javadoc excerpt of org.eclipse.e4.ui.model.application.ui.advanced.MPlaceholder.java(27-30) (Eclipse V4.15)):

...
 * A Placeholder is a concrete class used to share elements between perspectives. The
 * elements referenced by a Placeholder generally exist in the Window's 'sharedElements'
 * list. By convention a placeholder usually shares the same elementId as the element
 * that it's referencing.
...

Is there some documentation of the precise details concerning this ID issue? IMHO it is a bit unsatisfactory because I'm missing a comprehensible rule set when to do what.

How should we settle this issue? I would prefer, to reenable the same IDs for placeholders and their referenced elements, because it is documented this way. Or for consistency reasons to force different IDs for all placeholder / element pairs. The second solution has the weakness, that we probably will not be able to find and update all existing documentation, where the Javadoc above is cited in some way.

[Iā€™m wondering: the fixes for Bug 509644 are over three years old - am I really the first one who stumbled into this?]
Comment 4 Rolf Theunissen CLA 2020-04-25 15:00:29 EDT
The handling of non-unique elements is wrong in the ModelAssembler that merges the fragment elements into the main model. So the behavior of Eclipse is wrong here.

Fragments are not that commonly used, especially not to define the main structure of the application. Therefore, nobody stumbled on this issue yet. There are more known bugs related to merging fragments into the application

Why do you model large parts (the full?) application in fragments? You could define the main structure in the Application or Perspective. Then you would not need the merging of all the fragments.
Comment 5 Gerhard Kreuzer CLA 2020-04-26 04:50:17 EDT
Rolf, thanks for your quick reply. In order to understand the reasoning behind the design and architecture of the cited app, I must give some context information.
The app I'm speaking of is kind of engineering tool / IDE for our engineering staff. There are quite a few users spread over different departments and therefore there are a lot request and requirements concerning the functionality. Some time ago the idea was born, that we provide a kind of highly modular basic app (i.e. "toolkit") to enable "community development" ("We provide the means, so that you can help yourself.")
When looking out for the basic technology, we decided to use Eclipse RCP and when we started to design and code our tool (at the time of Eclipse Oxygen), we were not sure, whether we should build on pure E4 technology or include the compat layer. Your decision at that time was to start out with pure E4, but to develop in a way, so that we could easily switch to a compat-layer based app (if we should encounter the necessity to include a E3 based plug-in or alike). The aim was, that only the application providing  plug-in itself should have to be exchanged in order to switch between E4 and E3.
These decisions led to our app as it is. It is still pure E4 and will presumably stay so, but the highly modular design with clear separation of concerns (which also led to a heavy use of model fragments) proved as benefit.
Furthermore, our released app is still bound to Eclipse Oxygen, because it costs some effort to switch the basic framework (due to internal restrictions). But to be prepared, we recently decided to always port to the latest framework version and maintain our code base in a way to support the current released product and the latest framework version. The goal here is to have a single source base where only the manifest files and product definition should need to be adapted.
Currently I have no severe problems in realizing our goals with the current framework, but I hope to help to improve the overall quality of the Eclispe framework, by reporting peculiarities we encounter along the way.
Comment 6 Thomas Schindl CLA 2020-04-27 04:48:11 EDT
I agree with Rolf, this is a regression and needs to be fixed by the platform. The good thing is that you have a work around for the time being.