Bug 546932 - IDE looses basic key bindings with custom fragment.e4xmi with a bindingTable model fragment
Summary: IDE looses basic key bindings with custom fragment.e4xmi with a bindingTable ...
Status: REOPENED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 4.10   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Rolf Theunissen CLA
QA Contact:
URL:
Whiteboard:
Keywords:
: 455517 549103 (view as bug list)
Depends on:
Blocks:
 
Reported: 2019-05-02 10:53 EDT by Andreas Sewe CLA
Modified: 2020-06-04 11:17 EDT (History)
9 users (show)

See Also:


Attachments
Export CSV of General > Keys before Restore Defaults (31.38 KB, text/csv)
2019-05-03 06:34 EDT, Andreas Sewe CLA
no flags Details
Export CSV of General > Keys after Restore Defaults (31.38 KB, text/csv)
2019-05-03 06:35 EDT, Andreas Sewe CLA
no flags Details
Trace file for scenario in comment 10 (121.83 KB, text/x-log)
2019-05-06 03:29 EDT, Andreas Sewe CLA
no flags Details
Example project to reproduce the issue (4.05 KB, application/zip)
2019-05-16 10:18 EDT, Andreas Sewe CLA
no flags Details
Updated example project that works around the issue (4.01 KB, application/zip)
2019-05-17 08:25 EDT, Andreas Sewe CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Andreas Sewe CLA 2019-05-02 10:53:54 EDT
Eclipse looses many of its key bindings, e.g., Ctrl+C, Ctrl+V and Ctrl+Space. One has to go to the General > Keys and Restore Defaults to be able to continue working as normal.

This is obviously very annoying and happens for me and my colleagues quite frequently and at least since 2018-12 (but still with the latest 2019-06 M1). Some colleagues even switched to IntelliJ because of this bug.

I suspect that switching between applications triggers it, but I cannot reproduce it reliably.

If you have any advice on how to debug this, I would be glad to provide you with the necessary information.
Comment 1 Andreas Sewe CLA 2019-05-02 11:16:36 EDT
FYI, this occurs under both Linux (Ubuntu, ArchLinux with Gnome) *and* macOS.
Comment 2 Dani Megert CLA 2019-05-02 11:28:51 EDT
Are they lost during a session or on restart?

Anything in the .log?

Are the key bindings shown in the menu?

Where do you see it? Only editors (which ones) or also in entry fields and other areas?


Enable tracing with options you find in /org.eclipse.ui/.options
Comment 3 Andrey Loskutov CLA 2019-05-02 11:40:15 EDT
See bug 385278 comment 42. Probably this comment (but not the entire bug 385278) is the same problem. 

Question : is the *binding* lost completely, so that you really do not see any shortcut configured under the Keys preference page?
Comment 4 Andreas Sewe CLA 2019-05-03 06:33:25 EDT
Further infos:

It is not always the same key bindings which are lost.

Now, Ctrl+Z (Undo), Ctrl+Space (Content Assist) and Ctrl+1 (Quick Fix) are gone, but Ctrl+C/V (Copy & Paste) work just fine.

> Are they lost during a session or on restart?

During a session. IIRC, I used at least Ctrl+Z and Ctrl+Space successfully in the same session before.

> Anything in the .log?

Only this, which seems unrelated:

org.eclipse.core.runtime.CoreException: No property tester contributes a property org.eclipse.jdt.launching.jdtstratum to type class org.eclipse.jface.text.TextSelection

> Are the key bindings shown in the menu?

Yes.

Edit > Undo Typing has Ctrl+Z (and works as expected, if I click on it).

> Where do you see it? Only editors (which ones) or also in entry fields and other areas?

IIRC, only in editors (Java Editor, Build Properties Editor, Maven POM Editor, probably all of them)

> Question : is the *binding* lost completely, so that you really do not see any shortcut configured under the Keys preference page?

No, General > Keys shows the shortcuts for Undo and Content Assist; they just don't do anything. However, _Restore Defaults_ fixes the problem temporarily. So it must do *something*.

I'll hence attach the exported key bindings (Export CSV) before and after the _Restore Default_. Maybe you can spot what changed.
Comment 5 Andreas Sewe CLA 2019-05-03 06:34:50 EDT
Created attachment 278482 [details]
Export CSV of General > Keys before Restore Defaults
Comment 6 Andreas Sewe CLA 2019-05-03 06:35:08 EDT
Created attachment 278483 [details]
Export CSV of General > Keys after Restore Defaults
Comment 7 Dani Megert CLA 2019-05-03 07:55:36 EDT
> Enable tracing with options you find in /org.eclipse.ui/.options
Did you try that?
Comment 8 Andrey Loskutov CLA 2019-05-05 12:36:35 EDT
(In reply to Andreas Sewe from comment #4)
> Further infos:
> No, General > Keys shows the shortcuts for Undo and Content Assist; they
> just don't do anything. However, _Restore Defaults_ fixes the problem
> temporarily. So it must do *something*.
> 
> I'll hence attach the exported key bindings (Export CSV) before and after
> the _Restore Default_. Maybe you can spot what changed.

If you sort two attached files, you will see they are identical.

"Restore defaults" does this: org.eclipse.ui.internal.keys.model.KeyController.setDefaultBindings(IBindingService). This sets default scheme and removes all user defined bindings, and during that probably lot events are flying around.

All this reminds me on bug 527934 which I *personally* observed few times, *tried* to catch but haven't seen recently anymore.

If you observe this issue again, please enable tracing (to file, and set file size to > 10 MB).

Preferences -> General -> Tracing
Here in *both* "Platform e4 Workbench UI" and "Patform e4 Workbench SWT" set "debug", "trace", "trace/commands", "trace/focus" and "trace/eclipse.context" to "true". If it is too much, try to remove the noisiest options. You have to watch the output in order to find something useful, there will be LOT of output.

Also see bug 527934 comment 32, which is probably still relevant.

Andreas, on which OS are you? If it is Mac, can it be that this is somehow related to our Mac test fails/focus issues we see in the official SDK tests?
Comment 9 Andreas Sewe CLA 2019-05-06 03:16:21 EDT
(In reply to Andrey Loskutov from comment #8)
> All this reminds me on bug 527934 which I *personally* observed few times,
> *tried* to catch but haven't seen recently anymore.

Interesting. Bug 527934 comment 7 mentions that closing and re-opening the editor fixes the issue; will see whether this is the case here as well.
 
> If you observe this issue again, please enable tracing (to file, and set
> file size to > 10 MB).
> 
> Preferences -> General -> Tracing
> Here in *both* "Platform e4 Workbench UI" and "Patform e4 Workbench SWT" set
> "debug", "trace", "trace/commands", "trace/focus" and
> "trace/eclipse.context" to "true". If it is too much, try to remove the
> noisiest options. You have to watch the output in order to find something
> useful, there will be LOT of output.

Will do.

> Also see bug 527934 comment 32, which is probably still relevant.
> 
> Andreas, on which OS are you? If it is Mac, can it be that this is somehow
> related to our Mac test fails/focus issues we see in the official SDK tests?

I am on Ubuntu 18.04.2 LTS, with Gnome on X.org as desktop. A colleague has observed this on macOS, however.
Comment 10 Andreas Sewe CLA 2019-05-06 03:29:07 EDT
Created attachment 278500 [details]
Trace file for scenario in comment 10

Please find a trace.log attached, with options configured as per Andrey's suggestions in comment 8.

Here's what I did:

- Started the IDE
- In an already open editor, typed

  "".

-> This implicitly opened Content Assist
- Hit Escape to close Content Assist
- Hit Ctrl+Space to explicitly open Content Assist
-> This didn't work
- Opened General > Keys preferences and restored defaults
- Hit Ctrl+Space in the open editor again
-> This time, this opened Content Assist
Comment 11 Andreas Sewe CLA 2019-05-06 03:31:43 EDT
(In reply to Andreas Sewe from comment #10)
> - Hit Ctrl+Space to explicitly open Content Assist
> -> This didn't work

FYI, closing and reopening the editor doesn't fix the issue (see Bug 527934 comment 7).
Comment 12 Mickael Istria CLA 2019-05-06 04:37:28 EDT
So we have:
1. keybindings still declared (listed in menus) and visible in the Keys preference page
2. "Restore Defaults" fixing the bug
Am I right?

Could it be the `setScheme()` that's invoked in KeysPreferencePage fixing other things that the KeyBinding (for example fixing the current shortcut context or state in the KeyBinding manager)?

> -> This implicitly opened Content Assist
> - Hit Escape to close Content Assist
> - Hit Ctrl+Space to explicitly open Content Assist
> -> This didn't work
> - Opened General > Keys preferences and restored defaults
> - Hit Ctrl+Space in the open editor again
> -> This time, this opened Content Assist

I cannot reproduce the issue here with Fedora 29, but that really seems like an issue with context (ie closing the content-assist proposals removing or not restoring keybinding context).
Comment 13 Andreas Sewe CLA 2019-05-06 04:43:25 EDT
(In reply to Mickael Istria from comment #12)
> So we have:
> 1. keybindings still declared (listed in menus) and visible in the Keys
> preference page
> 2. "Restore Defaults" fixing the bug
> Am I right?

Correct.

> Could it be the `setScheme()` that's invoked in KeysPreferencePage fixing
> other things that the KeyBinding (for example fixing the current shortcut
> context or state in the KeyBinding manager)?
> 
> > -> This implicitly opened Content Assist
> > - Hit Escape to close Content Assist
> > - Hit Ctrl+Space to explicitly open Content Assist
> > -> This didn't work
> > - Opened General > Keys preferences and restored defaults
> > - Hit Ctrl+Space in the open editor again
> > -> This time, this opened Content Assist
> 
> I cannot reproduce the issue here with Fedora 29, but that really seems like
> an issue with context (ie closing the content-assist proposals removing or
> not restoring keybinding context).

Just checked that the implicitly opened Content Assist window is not to blame. Just typing a few spaces and then trying to do Ctrl+Z to "Undo Typing" doesn't work.
Comment 14 Andreas Sewe CLA 2019-05-06 05:09:34 EDT
Occurs under Windows 10 and with the latest SDK I-build (I20190505-1800), too.
Comment 15 Andreas Sewe CLA 2019-05-06 07:42:58 EDT
(In reply to Andreas Sewe from comment #14)
> Occurs under Windows 10 and with the latest SDK I-build (I20190505-1800),
> too.

OK, I think I've discovered how to trigger the bug: It occurs only in conjunction with a plugin (Teamscale Integration for Eclipse, <https://marketplace.eclipse.org/content/teamscale-integration-eclipse>) I myself co-develop and which, for dog-fooding purposes, is installed in all my Eclipses.

I recently added a fragment.e4xmi with a bindingTable model fragment to this plug-in; this is the exact change to which I can trace back this bug.

The question is now where the root cause and hence responsibility lies: Am I simply using the fragment.e4xmi wrong or is there a problem on the E4 side (in the compatibility layer)?

The relevant sections of my plugin are as follows:

plugin.xml:

  <extension point="org.eclipse.e4.workbench.model" id="com.teamscale.ide.eclipse.model">
   <fragment uri="fragment.e4xmi" apply="notexists"/>
  </extension>

fragment.e4xmi:

  <imports xsi:type="commands:BindingContext" xmi:id="_kaa3wBgLEemcCNGHw_7XJw" elementId="org.eclipse.ui.contexts.dialogAndWindow"/>

  <fragments xsi:type="fragment:StringModelFragment" xmi:id="_biaxwBgKEemcCNGHw_7XJw" featurename="bindingTables" parentElementId="xpath:/">
    <elements xsi:type="commands:BindingTable" xmi:id="_eY9hEBgKEemcCNGHw_7XJw" elementId="com.teamscale.ide.eclipse.bindingTables.teamscale" bindingContext="_kaa3wBgLEemcCNGHw_7XJw">
      <bindings xmi:id="_mVSecBgKEemcCNGHw_7XJw" elementId="com.teamscale.ide.eclipse.bindingTables.teamscale#uploadProjectsForPreCommit" keySequence="M1+9" command="_y7REUBgGEemcCNGHw_7XJw"/>
    </elements>
  </fragments>

As soon as I delete the bindingTables model fragment, the bug is gone (along with the binding, of course).

Another interesting observation: When installing the plug-in and doing the obligatory restart, the bindings work as expected. Only after the *second* restart does the bug show up. Any idea why?
Comment 16 Andreas Sewe CLA 2019-05-14 05:51:55 EDT
> The question is now where the root cause and hence responsibility lies: Am I simply using the fragment.e4xmi wrong or is there a problem on the E4 side (in the compatibility layer)?

I know that this bug *might* be NOT_ECLIPSE in that I may have done something wrong in my fragment.e4xmi. I don't think I did, though, so I was wondering what I could do from my side to help investigate this further? Provide a small example plug-in to trigger the bug perhaps?
Comment 17 Andrey Loskutov CLA 2019-05-15 16:39:06 EDT
(In reply to Andreas Sewe from comment #16)
> > The question is now where the root cause and hence responsibility lies: Am I simply using the fragment.e4xmi wrong or is there a problem on the E4 side (in the compatibility layer)?
> 
> I know that this bug *might* be NOT_ECLIPSE in that I may have done
> something wrong in my fragment.e4xmi. I don't think I did, though, so I was
> wondering what I could do from my side to help investigate this further?
> Provide a small example plug-in to trigger the bug perhaps?

A reproducer project is always good. But I can honestly say, as soon as I've read "my fragment.e4xmi" the bug was automatically removed from my mental (and pretty long) todo list. If you need a fix, try to provide a patch. This is the best way to fix this bug.
Comment 18 Andreas Sewe CLA 2019-05-16 10:18:30 EDT
Created attachment 278630 [details]
Example project to reproduce the issue

(In reply to Andrey Loskutov from comment #17)
> A reproducer project is always good.

You can find one attached, based on the "Menu contribution on 4.x API" skeleton created by the "New Plug-in Project" wizard. The only additions/changes are as follows:

- In plugin.xml, changed fragment/@apply from "initial" to "notexists"
- In fragment.e4xmi, added bindingTables model fragment and "org.eclipse.ui.contexts.dialogAndWindow" binding context import.

To trigger the bug, do the following:

- Import the Example project
- Run As > Eclipse Application (in a clear workspace)
  - Observe that E4 Handlers > Hello World has a short cut attached
  - Ctrl+9 opens a hello world dialog.
  - Create a Java project with a class
  - Trigger content assist there and observe that the shortcut works
- Run As > Eclipse Application again (but don't clear the workspace)
  - Observe that E4 Handlers > Hello World still has a short cut attached
  - Trigger content assist in the class and observe that the shortcut does *not* work

> But I can honestly say, as soon as I've
> read "my fragment.e4xmi" the bug was automatically removed from my mental
> (and pretty long) todo list. If you need a fix, try to provide a patch. This
> is the best way to fix this bug.

It's a bit sad that we are at Eclipse 4.11 now and writing IDE plug-ins in E4-style still is fraught with problems.

That being said, my primary concern is finding a workaround to this issue that works right now, not fixing this upstream for the future. Yes, that would benefit everyone in the long run, but I can't ignore that my users are having this problem right now with some Eclipse 4.11 or older.

But I will be sure to document my workaround in this ticket, in case it helps others or gives a hint as to what is going wrong here.
Comment 19 Andreas Sewe CLA 2019-05-16 10:51:05 EDT
FYI, with the E4 Model Spy, I observe that the org.eclipse.ui.contexts.dialogAndWindow Binding Context looses its child context (In Windows, In Dialogs) when the bug occurs. This would explain why some shortcuts still work, while others targetting these two don't.
Comment 20 Andreas Sewe CLA 2019-05-17 05:44:38 EDT
(In reply to Andreas Sewe from comment #19)
> FYI, with the E4 Model Spy, I observe that the
> org.eclipse.ui.contexts.dialogAndWindow Binding Context looses its child
> context (In Windows, In Dialogs) when the bug occurs. This would explain why
> some shortcuts still work, while others targetting these two don't.

Alas, when you then restart Eclipse, the SpyProcessor of the E4 Spies plugin does something to the org.eclipse.ui.contexts.dialogAndWindow binding context so that all shortcuts work again as expected. This makes debugging this with the Model Spy a bit cumbersome.
Comment 21 Andreas Sewe CLA 2019-05-17 08:02:58 EDT
(In reply to Andreas Sewe from comment #18)
> But I will be sure to document my workaround in this ticket, in case it
> helps others or gives a hint as to what is going wrong here.

I think I have found a workaround:

Rather than importing a BindingContext with ID "org.eclipse.ui.contexts.dialogAndWindow" and adding a bindingTables model fragment with a BindingTable whose context ID is "org.eclipse.ui.contexts.dialogAndWindow", I tried the following:

Import a BindingTable(!) with ID "org.eclipse.ui.contexts.window" and, in a model fragment, add KeyBinding to the bindings feature with ID "org.eclipse.ui.contexts.window". This seems to work so far.

This approach has the added benefit of working around Bug 469595, which prevents referencing "org.eclipse.ui.contexts.window" rather than the broader "org.eclipse.ui.contexts.dialogAndWindow" as a BindingContext in an import.

Hope this is helpful to others.
Comment 22 Andrey Loskutov CLA 2019-05-17 08:13:53 EDT
(In reply to Andreas Sewe from comment #21)
> (In reply to Andreas Sewe from comment #18)
> > But I will be sure to document my workaround in this ticket, in case it
> > helps others or gives a hint as to what is going wrong here.
> 
> I think I have found a workaround:

Andreas, I'm not sure I understand all of the changes, but could you attach an *updated* example plugin (without deleting the old one), so that one could see the difference?
Comment 23 Andreas Sewe CLA 2019-05-17 08:25:00 EDT
Created attachment 278639 [details]
Updated example project that works around the issue

(In reply to Andrey Loskutov from comment #22)
> (In reply to Andreas Sewe from comment #21)
> > I think I have found a workaround:
> 
> Andreas, I'm not sure I understand all of the changes, but could you attach
> an *updated* example plugin (without deleting the old one), so that one
> could see the difference?

Of course. Good idea.
Comment 24 Andreas Sewe CLA 2019-05-20 07:43:42 EDT
(In reply to Andreas Sewe from comment #21)
> I think I have found a workaround:

(In reply to Andreas Sewe from comment #23)
> Created attachment 278639 [details]

Did some further testing with the "Updated example project that works around the issue". The workaround works fine under Eclipse Neon or later, but fails under Mars, triggering a StackOverflowError in the MenuManager:

java.lang.StackOverflowError
        at java.util.HashMap.putVal(HashMap.java:629)
        at java.util.HashMap.put(HashMap.java:612)
        at java.util.HashSet.add(HashSet.java:220)
        at java.util.AbstractCollection.addAll(AbstractCollection.java:344)
        at java.util.HashSet.<init>(HashSet.java:120)
        at org.eclipse.jface.bindings.BindingManager.recomputeBindings(BindingManager.java:1720)
        at org.eclipse.jface.bindings.BindingManager.getPrefixTable(BindingManager.java:1520)
        at org.eclipse.jface.bindings.BindingManager.isPartialMatch(BindingManager.java:1605)
        at org.eclipse.jface.action.ExternalActionManager$CommandCallback.isAcceleratorInUse(ExternalActionManager.java:358)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:883)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:861)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:861)
        at org.eclipse.ui.internal.Workbench.updateActiveWorkbenchWindowMenuManager(Workbench.java:3413)
        at org.eclipse.ui.internal.Workbench.access$0(Workbench.java:3399)
        at org.eclipse.ui.internal.Workbench$1.bindingManagerChanged(Workbench.java:3394)
        at org.eclipse.jface.bindings.BindingManager.fireBindingManagerChanged(BindingManager.java:901)
        at org.eclipse.jface.bindings.BindingManager.setActiveBindings(BindingManager.java:2178)
        at org.eclipse.jface.bindings.BindingManager.recomputeBindings(BindingManager.java:1743)
        at org.eclipse.jface.bindings.BindingManager.getPrefixTable(BindingManager.java:1520)
        at org.eclipse.jface.bindings.BindingManager.isPartialMatch(BindingManager.java:1605)
        at org.eclipse.jface.action.ExternalActionManager$CommandCallback.isAcceleratorInUse(ExternalActionManager.java:358)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:883)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:861)
        at org.eclipse.jface.action.MenuManager.update(MenuManager.java:861)
Comment 25 Andreas Sewe CLA 2019-05-20 08:03:02 EDT
(In reply to Andreas Sewe from comment #24)
> Did some further testing with the "Updated example project that works around
> the issue". The workaround works fine under Eclipse Neon or later, but fails
> under Mars, triggering a StackOverflowError in the MenuManager:
> 
> java.lang.StackOverflowError

FYI, this is exactly the same SOE as reported in Bug 318587. Unfortunately, the comments in that ticket seem to revolve around just the NPE along with the SOE, but not the SOE itself.
Comment 26 Andreas Sewe CLA 2019-05-20 08:19:01 EDT
(In reply to Andreas Sewe from comment #25)
> (In reply to Andreas Sewe from comment #24)
> > Did some further testing with the "Updated example project that works around
> > the issue". The workaround works fine under Eclipse Neon or later, but fails
> > under Mars, triggering a StackOverflowError in the MenuManager:
> > 
> > java.lang.StackOverflowError
> 
> FYI, this is exactly the same SOE as reported in Bug 318587. Unfortunately,
> the comments in that ticket seem to revolve around just the NPE along with
> the SOE, but not the SOE itself.

For some reason, this seems to be linked to Bug 490024. If I apply the workaround stated there (set an explicit label for the menu item rather than just setting one on the command), the SOE is gone. Weird, but good enough for me ATM.
Comment 27 Rolf Theunissen CLA 2019-05-20 11:11:03 EDT
(In reply to Andreas Sewe from comment #15)
> The relevant sections of my plugin are as follows:
> 
> plugin.xml:
> 
>   <extension point="org.eclipse.e4.workbench.model"
> id="com.teamscale.ide.eclipse.model">
>    <fragment uri="fragment.e4xmi" apply="notexists"/>
>   </extension>

> Another interesting observation: When installing the plug-in and doing the
> obligatory restart, the bindings work as expected. Only after the *second*
> restart does the bug show up. Any idea why?

You have adding the model fragment with apply="notexists", this means that the fragment is applied to the main model only the first time. On successive restarts of the IDE, the fragment is not applied, the persisted version of the model is used.
This results in two questions:
1. Does the problem occur if you start with '-clearPersistedState' option added to the command line (This removes the persisted E4 model so I expect it to behave the same as after the restart)?
2. What happens when you change apply to apply="always"?
Comment 28 Andreas Sewe CLA 2019-05-20 11:52:29 EDT
(In reply to Rolf Theunissen from comment #27)
> (In reply to Andreas Sewe from comment #15)
> > Another interesting observation: When installing the plug-in and doing the
> > obligatory restart, the bindings work as expected. Only after the *second*
> > restart does the bug show up. Any idea why?
> 
> You have adding the model fragment with apply="notexists", this means that
> the fragment is applied to the main model only the first time. On successive
> restarts of the IDE, the fragment is not applied, the persisted version of
> the model is used.
> This results in two questions:
> 1. Does the problem occur if you start with '-clearPersistedState' option
> added to the command line (This removes the persisted E4 model so I expect
> it to behave the same as after the restart)?

Starting with '-clearPersistedState' causes all key bindings to work as expect, no matter how often I restart, albeit at the price of lost workbench state.

> 2. What happens when you change apply to apply="always"?

This doesn't help. After the restart, the key bindings are gone, even though the fragment with the binding table is applied to the "org.eclipse.ui.contexts.dialogAndWindow" context again.
Comment 29 Rolf Theunissen CLA 2019-05-20 16:08:46 EDT
I have been digging around a bit, it seems that bindings are moved from one binding table to another binding table. The binding tables involved all point to the same binding context.
When the model-spy is installed, I see the bindings moved to the model-spy binding table. In case of the attached project, the bindings seem to disappear all together.

The only suspect I have that could be moving the bindings between tables is BindingService.createORupdateMKeyBinding, this should be investigated further.
Comment 30 Eclipse Genie CLA 2019-05-30 11:43:30 EDT
New Gerrit change created: https://git.eclipse.org/r/143067
Comment 31 Rolf Theunissen CLA 2019-05-30 11:52:45 EDT
(In reply to Eclipse Genie from comment #30)
> New Gerrit change created: https://git.eclipse.org/r/143067

This change should prevent that key-bindings are moved to other binding-tables, and get lost. This should fix this bug.

However, after applying this patch Bug 469595 is triggered. The model contains a binding context and a binding table with the same elementId, which cannot be handled by the import element.
Comment 32 Andreas Sewe CLA 2019-06-11 03:22:05 EDT
(In reply to Rolf Theunissen from comment #31)
> (In reply to Eclipse Genie from comment #30)
> > New Gerrit change created: https://git.eclipse.org/r/143067
> 
> This change should prevent that key-bindings are moved to other
> binding-tables, and get lost. This should fix this bug.

Great.

> However, after applying this patch Bug 469595 is triggered. The model
> contains a binding context and a binding table with the same elementId,
> which cannot be handled by the import element.

I think this is correct behaviour for now or at least better than the current state. Also, for many binding contexts and tables, this won't be a problem, as their IDs differ.
Comment 34 Till Brychcy CLA 2019-07-02 04:45:32 EDT
I get the following exception when launching a nested eclipse, that sounds related to this. Maybe this is some related code path, that need to be updated, too?

org.eclipse.e4.core.di.InjectionException: java.lang.IllegalArgumentException: Binding context org.eclipse.wst.sse.ui.structuredTextEditorScope does not match org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext
	at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:68)
	at org.eclipse.e4.core.internal.di.InjectorImpl.processAnnotated(InjectorImpl.java:998)
	at org.eclipse.e4.core.internal.di.InjectorImpl.internalInject(InjectorImpl.java:139)
	at org.eclipse.e4.core.internal.di.InjectorImpl.internalMake(InjectorImpl.java:408)
	at org.eclipse.e4.core.internal.di.InjectorImpl.make(InjectorImpl.java:345)
	at org.eclipse.e4.core.contexts.ContextInjectionFactory.make(ContextInjectionFactory.java:227)
	at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.createFromBundle(ReflectionContributionFactory.java:94)
	at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.doCreate(ReflectionContributionFactory.java:60)
	at org.eclipse.e4.ui.internal.workbench.ReflectionContributionFactory.create(ReflectionContributionFactory.java:37)
	at org.eclipse.e4.ui.internal.workbench.swt.E4Application.createE4Workbench(E4Application.java:289)
	at org.eclipse.ui.internal.Workbench.lambda$3(Workbench.java:580)
	at org.eclipse.core.databinding.observable.Realm.runWithDefault(Realm.java:338)
	at org.eclipse.ui.internal.Workbench.createAndRunWorkbench(Workbench.java:558)
	at org.eclipse.ui.PlatformUI.createAndRunWorkbench(PlatformUI.java:150)
	at org.eclipse.ui.internal.ide.application.IDEApplication.start(IDEApplication.java:155)
	at org.eclipse.equinox.internal.app.EclipseAppHandle.run(EclipseAppHandle.java:203)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.runApplication(EclipseAppLauncher.java:137)
	at org.eclipse.core.runtime.internal.adaptor.EclipseAppLauncher.start(EclipseAppLauncher.java:107)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:400)
	at org.eclipse.core.runtime.adaptor.EclipseStarter.run(EclipseStarter.java:255)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.eclipse.equinox.launcher.Main.invokeFramework(Main.java:660)
	at org.eclipse.equinox.launcher.Main.basicRun(Main.java:597)
	at org.eclipse.equinox.launcher.Main.run(Main.java:1468)
	at org.eclipse.equinox.launcher.Main.main(Main.java:1441)
Caused by: java.lang.IllegalArgumentException: Binding context org.eclipse.wst.sse.ui.structuredTextEditorScope does not match org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext
	at org.eclipse.e4.ui.bindings.internal.BindingTable.addBinding(BindingTable.java:166)
	at org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.defineBinding(BindingProcessingAddon.java:191)
	at org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.defineBindingTable(BindingProcessingAddon.java:178)
	at org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.defineBindingTables(BindingProcessingAddon.java:158)
	at org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.init(BindingProcessingAddon.java:105)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.eclipse.e4.core.internal.di.MethodRequestor.execute(MethodRequestor.java:58)
	... 27 more


With the debugger, is saw that in 
org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.createBinding(Context, MCommand, List<MParameter>, String, MKeyBinding),
binding.getTransientData().get(EBindingService.MODEL_TO_BINDING_KEY) returns a binding with contextId="org.eclipse.wst.sse.ui.structuredTextEditorScope",
but the id field of bindingContext is "org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext"
Comment 35 Rolf Theunissen CLA 2019-07-02 11:59:36 EDT
(In reply to Till Brychcy from comment #34)

> With the debugger, is saw that in 
> org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.
> createBinding(Context, MCommand, List<MParameter>, String, MKeyBinding),
> binding.getTransientData().get(EBindingService.MODEL_TO_BINDING_KEY) returns
> a binding with contextId="org.eclipse.wst.sse.ui.structuredTextEditorScope",
> but the id field of bindingContext is
> "org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext"

The id "org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext" seems odd, only shortly in 2017 context ids ended with ".bindingcontext", see the reverted commits in Bug 469595.

Are you launching a nested eclipse on a old workspace?
Do you see the same error when launching on a fresh workspace?
Comment 36 Till Brychcy CLA 2019-07-02 14:31:31 EDT
(In reply to Rolf Theunissen from comment #35)
> (In reply to Till Brychcy from comment #34)
> 
> > With the debugger, is saw that in 
> > org.eclipse.e4.ui.workbench.swt.util.BindingProcessingAddon.
> > createBinding(Context, MCommand, List<MParameter>, String, MKeyBinding),
> > binding.getTransientData().get(EBindingService.MODEL_TO_BINDING_KEY) returns
> > a binding with contextId="org.eclipse.wst.sse.ui.structuredTextEditorScope",
> > but the id field of bindingContext is
> > "org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext"
> 
> The id "org.eclipse.wst.sse.ui.structuredTextEditorScope.bindingcontext"
> seems odd, only shortly in 2017 context ids ended with ".bindingcontext",
> see the reverted commits in Bug 469595.
> 
> Are you launching a nested eclipse on a old workspace?
> Do you see the same error when launching on a fresh workspace?

Thanks for the quick reply.

I can see ".bindingcontext" in the workspace's workbench.xmi. I'm using this workspace since 4.6 to work with the eclipse master, so this is an explanation.

I can manually remove these lines.

Still the problem never appeared before your patch (and goes away if I revert it). Would it be hard to add some code, so this doesn't lead to an error?
Comment 37 Rolf Theunissen CLA 2019-07-03 04:09:52 EDT
(In reply to Till Brychcy from comment #36)

> Still the problem never appeared before your patch (and goes away if I
> revert it). Would it be hard to add some code, so this doesn't lead to an
> error?

The patch that introduced ".bindingcontext" post-fixes was reverted within one day, over 1.5 years ago. That patch was reverted because it caused regressions similar to the ones you experience now.

I will not add some code to fix this, as this makes the code unnecessary more complex and it is far more efficient if you manually change the workbench.xmi.
We can only support backward compatibility between releases, not for every (reverted) patch in the development process.
Comment 38 Till Brychcy CLA 2019-07-03 04:31:34 EDT
(In reply to Rolf Theunissen from comment #37)
> (In reply to Till Brychcy from comment #36)
> 
> > Still the problem never appeared before your patch (and goes away if I
> > revert it). Would it be hard to add some code, so this doesn't lead to an
> > error?
> 
> The patch that introduced ".bindingcontext" post-fixes was reverted within
> one day, over 1.5 years ago. That patch was reverted because it caused
> regressions similar to the ones you experience now.
> 
> I will not add some code to fix this, as this makes the code unnecessary
> more complex and it is far more efficient if you manually change the
> workbench.xmi.
> We can only support backward compatibility between releases, not for every
> (reverted) patch in the development process.

OK for me - Actually I simply removed the workbench.xmi.
Comment 39 Rolf Theunissen CLA 2019-07-22 06:50:45 EDT
*** Bug 455517 has been marked as a duplicate of this bug. ***
Comment 40 Niraj Modi CLA 2019-07-25 06:20:36 EDT
*** Bug 549103 has been marked as a duplicate of this bug. ***
Comment 41 Andrey Loskutov CLA 2019-10-18 09:54:23 EDT
(In reply to Eclipse Genie from comment #33)
> Gerrit change https://git.eclipse.org/r/143067 was merged to [master].
> Commit:
> http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=fef8ea0602c3ae5c2e422300ee49760fcbee02bc

This causes severe regression, see bug 552010. I plan to revert this commit since the bug 552010 affects all users, and this one affects only a subset of users (that use extra xmi fragments with keybinding,  see comment 15).
Comment 42 Eclipse Genie CLA 2019-10-18 09:59:48 EDT
New Gerrit change created: https://git.eclipse.org/r/151309