-
Notifications
You must be signed in to change notification settings - Fork 389
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BIRT 4.16 202405030536, Designer-Crashed: topic depended on JRE 17 vs. 21 #1665
Comments
GEF has been making lots of changes lately. It seems to me we already dealt with a problem like this in the recent past. I have limited capacity this week to investigate problems. |
If you do Help -> Perform Setup Tasks... you will update the target platform and see this problem. The problem is a result of this commit: Which is for this issue: It happens here: org.eclipse.birt.report.designer.internal.ui.editors.rulers.EditorGuideEditPart.updateLocationOfFigures(int) Apparently that should now be a Rectangle, but I have no idea the design intent here in BIRT, i.e., how to convert the position to a rectangle. I'm not sure this new exception throwing is okay. The API being called has no constraint on the constraint: Here are other places it's called by BIRT: Probably the x-ed out one is the only bogus one... Commenting out that line seems to fix the problem; maybe this never did anything and was always just quietly ignore. |
I'm a little bit confused. I see a call to The reason we now thrown an exception when calling
I can have a look this evening, but it looks a lot like the wrong layout is assigned to the edit part. |
FYI, I'm traveling this week so my connectivity will be spotty at best. |
The EditorRulerEditorPart (or the EditorRulerFigure, to be more specific) is using the EditorRulerLayout, which extends the XYLayout. Because it overwrites the
|
Given that I can't predict how many other applications might be doing something similar, it probably makes sense to simply log an error/warning, rather than throwing an exception. This way clients still get feedback that something fishy is going on, without immediately breaking the editor. @azoitl wdyt? |
@ptziegler Given the legacy that is out I think this is a better solution. I also had a look what we could do to make the live for our users easier. Because I still strongly think that BIRT should not have used One thing that we can do for people that are doing similar things is to introduce a new |
My impression is that BIRT effectively copy-pasted the RulerLayout class to avoid exactly this problem. Which is why the EditorRulerLayout extends XYLayout, as this also used to be the case for the original class.
Makes sense to me. |
With this GEF Classic PR the quickest and most correct fix is to change the parent class of |
In addition, I've also created eclipse/gef-classic#440, which fixes this problem without the need to adapt BIRT. Instead of throwing an exception, the layouts now simply create a warning and add them to the error log. |
I'm switching the target platform to the nightly build: Indeed it only logs now. What should be properly do to actually fix the problem? I don't understand what the constraint's purpose is in the BIRT usage.... |
Instead of extending the |
That's not something I can reproduce. I also see that there is a noticable difference in the timestamps. Are you certain that those aren't just the warnings created from before your change?
This only removes the warnings for the I think this warning is accurate and it highlights a problem that can very easily lead to a ClassCastException. Because
Furthermore, this layout is used within the
My suggestion is repeat the same thing for the |
Yes, I saw same classes on my debug session some minutes ago. |
Summary of the 3 extended changes and I agree to solve all topics there is more to after the replacement: EditorRulerLayout
TableLayout
TableXYLayoutEditPolicy
|
switch from XYLayoutEditPolicy to ConstrainedLayoutEditPolicy (eclipse-birt#1665)
TableLayout already has a method called
From what I understand, this edit policy is only used to handle resizing the table cells. It's probably fine to leave them as dummy implementations (similar to most of the other methods). |
I have created a first PR #1678 and added the missing methods accordingly the original sources to avoid behavior changes. My be we can change these according to your suggestion as dummy-methods but it is the first time that I'm so deep into the designer-engine level and therefore the pesimistic implementation of the copied methods. @merks What do you think about it? |
* Fix BIRT-designer, switch from XYLayout to AbstractConstraintLayout and switch from XYLayoutEditPolicy to ConstrainedLayoutEditPolicy (#1665)
The change is merged to the BIRT-master. |
The All-In-One-designer is crashed with the currently "nightly build".
Tested designer versions from my side 4.16:
» N202405030536: 4.16 designer crashed
» N202404301804: 4.16 ok
» N202404291807: 4.16 ok
» N202404271127: 4.16 ok
» N202404261927: 4.16 ok
Could it be a topic with the latest eclipse changes an the required version of JRE 21 for some libraries or some central changes at eclipse-framwork side?
I have no problems with my dev-environment locally.
Log: log.zip
Designer
Error-Screen 01, 1st message
Error-Screen 02, 2nd message
The text was updated successfully, but these errors were encountered: