core: fix window.menuBarVisibility bug on startup

[vince-fugnitto]

What it does

The following wip pull-request exposes an issue previously reported on our forum. When starting the application, we end up in a state when the top-level menu (main menu) is not visible:

How to test

1. clean environment (rm -rf ~/.theia)
2. build the example-browser application
3. start the application, and notice the missing top-level menu

Review checklist

• As an author, I have thoroughly tested my changes and carefully followed the review guidelines

Reminder for reviewers

• As a reviewer, I agree to behave in accordance with the review guidelines

Signed-off-by: vince-fugnitto [email protected]

[vince-fugnitto]

I confirmed that the issue seems to be caused by #9830.

Using the steps described by the pull-request, I was able to reproduce the issue on 6f80dc1 but not on the previous 5cc8eaf commit.

[msujew]

Adding:

this.showMenuBar(menuBar, this.corePreferences['window.menuBarVisibility']);

Somewhere in here:

theia/packages/core/src/browser/menu/browser-menu-plugin.ts

Lines 58 to 61 in 1f1aecb

	createMenuBar(): MenuBarWidget {
	const menuBar = new DynamicMenuBarWidget();
	menuBar.id = 'theia:menubar';
	const preferenceListener = this.corePreferences.onPreferenceChanged(preference => {

Does the trick. #9830 Seemed to remove the fillMenuBar call that existed previously.

[vince-fugnitto]

Does the trick. #9830 Seemed to remove the fillMenuBar call that existed previously.

Same thing I tried locally :)

[vince-fugnitto]

I'll drop the first commit (which exposes the issue) when approved 👍

[colin-grant-work]

This works for me and follows the correct pattern for preference-conditioned startup. 👍

[msujew]

I can confirm as well. The fix addresses the issue nicely 👍

[colin-grant-work]

@vince-fugnitto, it seems that this fix only solves the issue for the browser platform but not Electron, where the modified code doesn't run. I think it was coincidence that the application startup in Electron ran in such a way that a listener was attached in time to catch the initial preference setting; downstream, we're seeing that that isn't always the case.