fix: memory leak in editor edit context

[SimonSiefke]

Register the screenReaderSupport as a disposable.

[aiday-mar]

Hi thanks for making the PR. The ScreenReaderSupport class is not an IDisposable nor a Disposable. I am not sure why we register the class, as it does not have a dispose method anyway. Could you please let me know?

[SimonSiefke]

Hi,
Thanks for the feedback. Checking ScreenReaderSupport.ts shows the ScreenReaderSupport extends Disposable:

export class ScreenReaderSupport extends Disposable {

	// Configuration values
	private _contentLeft: number = 1;
	private _contentWidth: number = 1;

It seems to me it should be registered in native edit context similar to focusTracker

export class NativeEditContext extends AbstractEditContext {
	constructor(/* ... */) {
		super(context);
        /* ... */

        // focus tracker is being registered
		this._focusTracker = this._register(new FocusTracker(this.domNode.domNode, (newFocusValue: boolean) => {
			this._screenReaderSupport.handleFocusChange(newFocusValue);
			this._context.viewModel.setHasFocus(newFocusValue);
		}));
       
        // screen reader support is not registered (?)
		this._screenReaderSupport = instantiationService.createInstance(ScreenReaderSupport, this.domNode, context, this._viewController, this);

What do you think?

[aiday-mar]

Hi @SimonSiefke yes absolutely you are right. I was looking at the code on an older branch that had not yet made the change to make it disposable apologies for that. I will approve the PR.

[ivanstepanovftw]

Hello! Please, review and merge #154465

[aiday-mar]

Hi @ivanstepanovftw thanks for the comment. I am not the owner of that feature area unfortunately so am not apt for approving the PR.