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.

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

workbenchThemeService.setColorTheme disposes themingParticipantChangeListener #27097

Closed
nicksnyder opened this issue May 22, 2017 · 3 comments
Closed
Assignees
Labels
bug Issue identified by VS Code Team member as probable bug themes Color theme issues verified Verification succeeded
Milestone

Comments

@nicksnyder
Copy link
Contributor

I am building a component that is lazily loaded (and therefore lazily calls registerThemingParticipant). The callback passed in to registerThemingParticipant is never getting called though and it seems like this is because workbenchThemeService.setColorTheme disposes any previous themingParticipantChangeListener and does not add it back for some code paths.

It looks like setColorTheme assumes that applyTheme will add back the listener here, but there are multiple code paths in setColorTheme that neither call setColorTheme nor add back the listener.

The specific case I am hitting is
https://sourcegraph.com/github.com/Microsoft/vscode/-/blob/src/vs/workbench/services/themes/electron-browser/workbenchThemeService.ts#L411

if (themeId === this.currentColorTheme.id && !this.currentColorTheme.isLoaded && this.currentColorTheme.hasEqualData(themeData)) {
	// the loaded theme is identical to the perisisted theme. Don't need to send an event.
	this.currentColorTheme = themeData;
	themeData.setCustomColors(this.colorCustomizations);
	return TPromise.as(themeData);
}

When this case is hit, the listener is never re-registered, so subsequent calls to registerThemingParticipant will have no effect.

@bpasero bpasero assigned aeschli and unassigned bpasero May 23, 2017
@aeschli aeschli added this to the May 2017 milestone May 23, 2017
@aeschli aeschli added bug Issue identified by VS Code Team member as probable bug themes Color theme issues labels May 23, 2017
@sandy081 sandy081 added the verified Verification succeeded label Jun 1, 2017
@sandy081
Copy link
Member

sandy081 commented Jun 1, 2017

@aeschli I do not see this is an extension API.. are there any steps to verify this?

@sandy081 sandy081 removed the verified Verification succeeded label Jun 1, 2017
@aeschli
Copy link
Contributor

aeschli commented Jun 1, 2017

No, its just in code, no need to verify

@sandy081 sandy081 added the verified Verification succeeded label Jun 1, 2017
@sandy081
Copy link
Member

sandy081 commented Jun 1, 2017

Adding verified tag so that it wont be seen in the to verify list

@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue identified by VS Code Team member as probable bug themes Color theme issues verified Verification succeeded
Projects
None yet
Development

No branches or pull requests

4 participants