fix: memory leak in editor edit context#256957
Conversation
|
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? |
|
Hi, 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 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? |
|
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. |
|
Hello! Please, review and merge #154465 |
|
Hi @ivanstepanovftw thanks for the comment. I am not the owner of that feature area unfortunately so am not apt for approving the PR. |
Register the
screenReaderSupportas a disposable.