Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[electron] Multiple electron menus/appIcons when starting the electron example #5385

Closed
kittaakos opened this issue Jun 6, 2019 · 3 comments · Fixed by #5400
Closed

[electron] Multiple electron menus/appIcons when starting the electron example #5385

kittaakos opened this issue Jun 6, 2019 · 3 comments · Fixed by #5400
Labels
bug bugs found in the application electron issues related to the electron target

Comments

@kittaakos
Copy link
Contributor

Description

I can see multiple application icons on my Dock on macOS when I start the electron example.

Screen Shot 2019-06-06 at 21 53 53

Each has a different application menu.

  1. The visible:
    Screen Shot 2019-06-06 at 21 56 54

  2. The "hidden?":
    Screen Shot 2019-06-06 at 21 56 47

Reproduction Steps

yarn --cwd ./examples/electron/ start

OS and Theia version:
macOS

0b1b853

It is just a wild guess but #5360 could be related. We start something else we did not do before. Smells like a regression.
@kittaakos kittaakos added bug bugs found in the application electron issues related to the electron target labels Jun 6, 2019
@kittaakos
Copy link
Contributor Author

Each time I refresh the browser window, I have a new app icon on the Dock:
Screen Shot 2019-06-06 at 22 14 44

@kittaakos kittaakos mentioned this issue Jun 6, 2019
Merged
@vince-fugnitto
Copy link
Member

I get the same issue, I tried removing plugin-dev (the source of the hosted-plugin-service) from the example-electron application, and it had no effect. Moreover, I removed all plugin packages and it also had no effect so the root cause must be something else.

@kittaakos kittaakos mentioned this issue Jun 7, 2019
Closed
@vince-fugnitto
Copy link
Member

vince-fugnitto commented Jun 7, 2019 •
edited
Loading

@kittaakos
I tested and it looks like I can only reproduce the bug on macOS. I've tested on Linux (Ubuntu 16.04) and I did not see the fault. I have yet to test on Windows but I just wanted to update you.

Edit
Windows does not have the issue.

kittaakos pushed a commit that referenced this issue Jun 7, 2019
GH-5385: Do not spawn electron for the TS LS.
9806c5b 
When spawning/forking a new node process with Node.js bundled into
electron, we have to explicitly disable the electron part.

Closes #5385.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos kittaakos mentioned this issue Jun 7, 2019
Merged
kittaakos referenced this issue Jun 7, 2019
@marcdumais-work
[typescript] re-use Theia launching program for TS LS
de45794 
When we launch the TypeScript Language Server, we start it using
the hardcoded executable "node". This works well in practice for
most cases. However when we have a packaged Electron application,
installed by a user, it could be that the environment does not have
"node" immediately available in its path, or not a proper version,
as needed by tsserver.

So why not reuse the node executable that was used to launch
the Theia application, to launch the LS? In the Electron case,
it will be the Electron executable bundled with the packaging.

Signed-off-by: Marc Dumais <marc.dumais@ericsson.com>
@kittaakos kittaakos mentioned this issue Jun 7, 2019
Merged
kittaakos pushed a commit that referenced this issue Jun 10, 2019
GH-5385: Do not spawn electron for the TS LS.
c7f036e 
When spawning/forking a new node process with Node.js bundled into
electron, we have to explicitly disable the electron part.

Adjusted the JSON LS server contribution to the TS one.

Closes #5385.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
kittaakos pushed a commit that referenced this issue Jun 10, 2019
@kittaakos
GH-5385: Do not spawn electron for the TS LS.
13df4a0 
When spawning/forking a new node process with Node.js bundled into
electron, we have to explicitly disable the electron part.

Adjusted the JSON LS server contribution to the TS one.

Closes #5385.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
marcdumais-work pushed a commit that referenced this issue Jun 13, 2019
@marcdumais-work
GH-5385: Do not spawn electron for the TS LS.
ff0adb3 
When spawning/forking a new node process with Node.js bundled into
electron, we have to explicitly disable the electron part.

Adjusted the JSON LS server contribution to the TS one.

Closes #5385.

Signed-off-by: Akos Kitta <kittaakos@typefox.io>
@kittaakos kittaakos mentioned this issue Jun 28, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug bugs found in the application electron issues related to the electron target
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

GH-5385: Do not spawn electron for the TS LS. eclipse-theia/theia
2 participants
@kittaakos @vince-fugnitto