Skip to content

Action labels missing from Undo/Redo menu items since 2023-09 (Eclipse 4.29.0) #1314

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

Closed
pcdavid opened this issue Nov 19, 2023 · 11 comments
Closed
Labels
bug Something isn't working regression

Comments

@pcdavid
Copy link

pcdavid commented Nov 19, 2023

Steps to reproduce:

  1. Open a plain text file.
  2. Type some text.
  3. Open the "Edit" menu.

Expected: the "Undo" entry is labeled "Undo Typing".

Actual result: the menu entry is simply labeled "Undo", with no reference to the action that will be undone (or redone, the same applies to the "Redo" menu entry).

Below: first screenshot on 2023-06, second on 2023-09.

2023-06
2023-09

I first saw this when a lot of our tests (in Sirius) failed on 2023-12, but it is not specific to Sirius or even anything related to modeling. As indicated above, I reproduce it with a plain text file.

On Eclipse 2023-06, this works as expected. The regression appears in 2023-09 (and is present in the upcoming 2023-12).

@pcdavid
Copy link
Author

pcdavid commented Nov 19, 2023

It seems to have been caused by #978.
Reverting WorkbenchActionBuilder to the version before that change fixes the issue for me (the action labels are back, e.g. "Undo Typing" and "Redo Typing").

@BeckerWdf
Copy link
Contributor

@N1k145: Can you pls. have a look at it and provide a fix for that regression?

@N1k145
Copy link
Contributor

N1k145 commented Nov 20, 2023

@BeckerWdf @pcdavid
I had a quick look and I'm able to reproduce this.
I have the feeling that this is somewhat unsupported by the other implementation, but I will have a look.
The change I made was basically switching the menu entry to a mechanism supporting E4 so that E4 contributions can provide their own handlers.

@merks
Copy link
Contributor

merks commented Nov 20, 2023

This issue was raised in the EMF forum too:

https://www.eclipse.org/forums/index.php/t/1113985/

N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Nov 20, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
@N1k145
Copy link
Contributor

N1k145 commented Nov 20, 2023

@pcdavid @BeckerWdf @merks
I analyzed the issue:
The old implementation (pure E3) did take the name of the Action and had a listener when the Action changes that refreshed the UI controls.

For this, there is currently no support in E4, which is used by the new implementation. A Handler does not provide a name, only a command.
I added a basic support for this case in the Compatibility Construct, with this a Handler could provide a Name that is queried when the handler is updated. Similar to the pure E3 case.

This currently only works for ActionHandlers but could be extended to be a general E4 concept.

I opened a draft PR with these changes, feel free to comment on this, it's not jet final as some documentation is still missing

N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Nov 20, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Nov 21, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Nov 21, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Nov 22, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
@jukzi jukzi added bug Something isn't working regression labels Nov 28, 2023
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Dec 1, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Dec 1, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
mickaelistria pushed a commit to N1k145/eclipse.platform.ui that referenced this issue Dec 4, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Dec 6, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Dec 7, 2023
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
@BeckerWdf
Copy link
Contributor

I think this is fixed. Isn't it?

@LorenzoBettini
Copy link
Contributor

In 2023-12?

@iloveeclipse
Copy link
Member

#1317 is still open

N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
N1k145 added a commit to N1k145/eclipse.platform.ui that referenced this issue Jan 30, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
mickaelistria pushed a commit that referenced this issue Jan 31, 2024
Fixes Regression: #1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
@mickaelistria
Copy link
Contributor

Hopefully fixed with #1317

@LorenzoBettini
Copy link
Contributor

@mickaelistria will it be available in tomorrow i-builds?

@mickaelistria
Copy link
Contributor

will it be available in tomorrow i-builds?

Yes, unless the build fails for some other reason.

amartya4256 pushed a commit to amartya4256/eclipse.platform.ui that referenced this issue Feb 8, 2024
Fixes Regression: eclipse-platform#1314

With this a basic support for named handlers is introduced, currently this is for compatibility between ActionHandlers and E4.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working regression
Projects
None yet
Development

No branches or pull requests

8 participants