Avoid FindReplaceOverlay setTextEditorActionsActivated resulting in NPE

[merks]

• The FindReplaceOverlay's setTextEditorActionsActivated method reflectively invokes AbstractTextEditor.setActionActivation(boolean) and that method assumes that getEditorSite().getActionBarContributor() returns non-null but MultiPageEditorSite.getActionBarContributor() returns null so in PDE's target editor, this leads to an NPE.
• AbstractTextEditor.setActionActivation(boolean) must guard against a null action contributor to avoid NPE
• The logic in setTextEditorActionsActivated already has special case handling for MultiPageEditorSite which should be augmented to call IEditorActionBarContributor.setActiveEditor(IEditorPart) on the overall multi-page editor.

[merks]

To reproduce the original problem open a *.target file with the PDE's Target Editor, turn to the Source tab and type Ctrl-F. I didn't even notice the logged errors initially, but I did notice that Ctrl-C didn't paste into the overlay but rather into the editor (and then I noticed that Revert didn't work for that editor, which is now fixed).

Anyway, I don't know if this is the right approach for fixing this. Also, given the careful order of actions in AbstractTextEditor.setActionActivation(boolean), the current change does not preserve that order/inverse-order for the setActiveEditor call and the processing of the actions. I think more refactoring would be needed if that order is important.

Please feel free to reuse this PR in any way, shape, or form, or to rewrite it.

[github-actions]

Test Results

 1 818 files  ±0   1 818 suites  ±0   1h 27m 51s ⏱️ - 3m 57s
 7 739 tests ±0   7 511 ✅ ±0  228 💤 ±0  0 ❌ ±0 
24 378 runs  ±0  23 629 ✅ ±0  749 💤 ±0  0 ❌ ±0 

Results for commit e90c9a6. ± Comparison against base commit cc5231d.

♻️ This comment has been updated with latest results.

[merks]

This solution is simpler and I think more correct.

[HeikoKlare]

Thank you, @merks!

I have tested your change and it works for me as expected. Given that we accept the hack of reflectively calling setActionActivation(boolean), I agree that this is probably the right thing to do. I do not see a risk of regression, as the only other "user" (in terms of reflective access) of the changed method in addition to the FindReplaceOverlay is JDT's breadcrumb for Java editors. Since they are usually not placed inside MultiPageEditors, they will not be affected at all. And for the FindReplaceOverlay we already have some special handling for other kinds of MultiPageEditors.

Interestingly, PDE's MultiPageEditor for bundle information (MANIFEST, build.properties etc.) and this one for target files seem to behave differently, as they require different mechanism to temporarily deactivate the editor's actions.

This kind of hacking into the action activation is rather unsatisfying, as it is difficult to make and keep it work properly (see also #2651 for a related issue). From my understanding, this is all rooted in the fact that such addition/overlay implementations need their own action scope but as they do not participate in the ordinary workbench/part model handling the action activation, it required custom mechanisms.

[fedejeanne]

@merks I'm not really familiar with this code, Heiko supervised this feature and knows the code.

[merks]

The failing test:

and the new warnings:

are not related to this change.