The first tab is not properly activated when opening the "Run configurations" dialog

[fedejeanne]

When opening the Run Configurations Dialog, the activated method of the first tab in the currently selected Run Configuration is not invoked.

How to reproduce

• Override the activated method in org.eclipse.jdt.debug.ui.launchConfigurations.JavaMainTab (just call the super implementation in it)
• Add a breakpoint to that method
• Start sdk.product in debug mode
• Create a new Java class with a main method and run it
• Open the Run Configurations dialog
  • --> The newly created run configuration should be selected (it's under Java Application)
  • --> The execution didn't stop at the breakpoint

[iloveeclipse]

• Is this regression in 4.30?
• If yes, it is a severe one? We plan for RC1 now, so does it need to be in RC1? If yes, please explain why & comment on Declare 4.30 RC1 eclipse.platform.releng.aggregator#1547
• What is the end-user impact of the problem? I could not really understand what "properly activated" is supposed to do.

[iloveeclipse]

OK, if I understand it right, this must be a regression in 4.30 from commit 96daf27
/ #766.

If I understand it right, the first shown tab of any launch configuration type may be not properly initialized now?

@HeikoKlare, @fedejeanne: this looks like a major regression for me & the fix should be in 4.30.

[fedejeanne]

OK, if I understand it right, this must be a regression in 4.30 from commit 96daf27 / #766.

It's not a regression but that commit was supposed to fix the same issue and it only did it partially: The UI looked good, everything was there, but the activated method was not being called. If you revert that commit in master, the issue still exists.

If I understand it right, the first shown tab of any launch configuration type may be not properly initialized now?

That is correct, which is why I would like to have this PR merged asap.

@HeikoKlare, @fedejeanne: this looks like a major regression for me & the fix should be in 4.30.

Again: this is not a regression but an already existing issue. The thing is that since not many tabs are making use of the activated method, this issue remained hidden until recently. It only became apparent when I started moving long-running operations to the activated methods of the tabs. You could say that #766 was the first failed attempt at solving this old hidden issue.

[iloveeclipse]

Again: this is not a regression but an already existing issue.

If this not a regression and activated was never called, I would wait then for 4.31 M1, except there is a very good reason / (other regression caused by the new code?) for the change to be in RC2.

[fedejeanne]

Again: this is not a regression but an already existing issue.

If this not a regression and activated was never called, I would wait then for 4.31 M1, except there is a very good reason / (other regression caused by the new code?) for the change to be in RC2.

The regression happened in eclipse-pde/eclipse.pde#674. The problem that caused that regression existed for a long time (it was not introduced recently) but #674 merely exposed it and it can be reproduced as follows:

• Create 2 PDE Run Configurations: "A" and "B"
• Open the Run Configurations Dialog
• Select "A" and switch to the "Tracing" tab
• Select "B"
  • --> The activated method will not be called (the UI looks fine though)

Previous regressions were added over the years (every time a tab overrides activated) and they can be reproduced in the exact same way as described above.

Since #674 was recently added, I think we should merge this PR before RC2.

[merks]

It seems to me this is probably worth re-spinning RC1 to ensure it's well tested before the final RC2 build...

[iloveeclipse]

The regression happened in eclipse-pde/eclipse.pde#674

Which regression? If I understood it right, not activating the first tab was always the case, according to @HeikoKlare ?
Do you mean, it affects "PDE Tracing" tab only?
Can you please describe exactly the regression, from user point of view - what would be broken if the Tracing tab "is not properly activated"?

@merks : the fix proposed here would affect ALL contributed launch configuration tabs, not only Tracing one from PDE, and I simply fear that "unexpected" tab activation that was never working before would cause more troubles for all kinds of custom launch configurations as the "not properly activated" Tracing tab in PDE, especially it is so short before release.

[HeikoKlare]

@merks : the fix proposed here would affect ALL contributed launch configuration tabs, not only Tracing one from PDE, and I simply fear that "unexpected" tab activation that was never working before would cause more troubles for all kinds of custom launch configurations as the "not properly activated" Tracing tab in PDE, especially it is so short before release.

I agree. It would even be an option to revert the PDE fix and reapply it for 4.31 to avoid any kind of regression, wouldn't it? As far as I understood, the bug did not produce any unintended behavior visible for the user before the PDE fix.

Which regression? If I understood it right, not activating the first tab was always the case, according to @HeikoKlare ?

@iloveeclipse Just to be sure that we do not rely on false assumptions: I cannot remember having mentioned that, as I have not analyzed that issue at all. Do you mean @fedejeanne?

[iloveeclipse]

@HeikoKlare : I maybe misunderstood this comment: #859 (comment)

Again: this is not a regression but an already existing issue. The thing is that since not many tabs are making use of the activated method, this issue remained hidden until recently.

[HeikoKlare]

@iloveeclipse Alright, that comment was by @fedejeanne 🙂

[iloveeclipse]

It would even be an option to revert the PDE fix and reapply it for 4.31 to avoid any kind of regression, wouldn't it?

I think this is a better option, and that could go into RC2 without RC1 respin. Can someone please prepare this PR & also give an assessment which effects it might have.

[fedejeanne]

It would even be an option to revert the PDE fix and reapply it for 4.31 to avoid any kind of regression, wouldn't it?

I think this is a better option, and that could go into RC2 without RC1 respin. Can someone please prepare this PR & also give an assessment which effects it might have.

That would be an option too. Sadly I won't be able to make it today since I need to leave the office. @HeikoKlare would you be so kind? :-)

[HeikoKlare]

I can "technically" prepare a reverting PR and say that from my understanding this will make performance of the launch configurations dialog worse (like it has been before the PDE fix). Since I was not involved in the PDE fix, I cannot make any further statements on the effects without taking some time to understand it in more detail. Maybe you can at least share your expectations w.r.t. that @fedejeanne? Or maybe @HannesWell can assess possible effects (except for the performance one), since he reviewed the PR?

[HeikoKlare]

I've created eclipse-pde/eclipse.pde#925 that contains a revert commit of the PDE fix.

[fedejeanne]

... Maybe you can at least share your expectations w.r.t. that @fedejeanne?

I only expected it to loose the performance gain and nothing else. Luckily, this was confirmed: I tested eclipse-pde/eclipse.pde#925 and it's looking good. Thank you for your help!