Hide Markdown preview editor icon when there is no active icon theme

[dvrylc]

This PR fixes #84962.

Approach

• Set the Markdown preview editor's icon only if there is an active icon theme
• Attach a handler for the onDidChangeConfiguration event so that preference changes are reflected live

I've seen in this comment that the issue shouldn't be fixed at the Markdown extension level but I still went with this approach because:

• I couldn't reproduce the bug in any other Webview
• This seems isolated to the Markdown extension's incorrect setting of the iconPath regardless of the workbench preferences

[mjbvz]

I still don't think this should be fixed at the markdown extension. Extensions should be free to set the icon to whatever they wish, but then VS Code should determine if it should display those icons or not

If we can't fix this with css rules, I recommend moving the logic into this class in core VS Code instead:

vscode/src/vs/workbench/contrib/webview/browser/webviewEditorInput.ts

Line 18 in a8158ae

class WebviewIconsManager {

[dvrylc]

I'm trying to move the logic into core but I'm not sure how to read configuration values from within the WebviewIconsManager. What's the usual approach for this?

https://github.com/microsoft/vscode/blob/master/src/vs/workbench/contrib/webview/browser/webviewEditorInput.ts#L29

[mjbvz]

I pushed d6fba31 that makes the change simpler.

Icon manager code:

vscode/src/vs/workbench/contrib/webview/browser/webviewService.ts

Line 21 in d6fba31

@IInstantiationService private readonly _instantiationService: IInstantiationService,

You'll want to use the IConfigurationService. It has a similar API to the external configuration api

[dvrylc]

Thanks for that! I have a good idea of how to fix this, but now I can't seem to reproduce the bug on the latest master. Can you confirm this?

6861571

[mjbvz]

Yes it's still broken:

[msftclas]

All CLA requirements met.

[dvrylc]

I've updated the PR with the following implementation:

• Check whether there is an active icon theme before setting it
• Force the Markdown preview webview to update each time there is a configuration change so that new options are reflected in the current open tabs

[dvrylc]

Thanks for the comments!

I refactored the approach to only check for an active icon theme when updating the stylesheet. This ensures that:

• Extensions can freely set the icon
• Icons are only shown when there is an active icon theme
• Configuration changes are immediately reflected

I do have a question however –

vscode/src/vs/workbench/contrib/webview/browser/webviewIconManager.ts

Line 40 in 3a1e245

private async updateStyleSheet(lifecycleService: ILifecycleService) {

Why does updateStyleSheet need to take lifecycleService as an argument, given that it's already passed in the constructor?

[mjbvz]

Only one more minor change needed. Otherwise looks good!

[dvrylc]

Alright, should be good to go!

[mjbvz]

Thanks! This will be in the next insiders build and will ship with VS Code 1.42