Skip to content

Commit a3e1281

Browse files
metastable-voidmetastable-void
authored
Merge pull request #609 from hajimemanila/fix/memory-leak-audit
Perf: Fix memory leaks via Event Delegation & Lifecycle cleanup (Fixes #600)
2 parents 9f94164 + 31a9062 commit a3e1281

5 files changed

Lines changed: 311 additions & 109 deletions

File tree

‎eslint-result.json‎

Lines changed: 1 addition & 0 deletions
Large diffs are not rendered by default.

‎src/components/menulist-container.ts‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable */
12
/* -*- indent-tabs-mode: nil; tab-width: 2; -*- */
23
/* vim: set ts=2 sw=2 et ai : */
34
/**
@@ -19,6 +20,7 @@
1920
@license
2021
**/
2122

23+
// eslint-disable-next-line @typescript-eslint/no-unused-vars
2224
import browser from 'webextension-polyfill';
2325
import { DisplayedContainer } from 'weeg-containers';
2426
import { EventSink } from "weeg-events";
@@ -32,6 +34,7 @@ export class MenulistContainerElement extends HTMLElement {
3234
private _tabCount = 0;
3335
private _hidden = false;
3436
private readonly _isPrivate;
37+
private readonly _cookieStoreId: string;
3538

3639
public readonly onContainerHide = new EventSink<void>();
3740
public readonly onContainerUnhide = new EventSink<void>();
@@ -52,6 +55,7 @@ export class MenulistContainerElement extends HTMLElement {
5255
throw new Error("Shadow root is null");
5356
}
5457
this._isPrivate = isPrivate || displayedContainer.cookieStore.isPrivate;
58+
this._cookieStoreId = displayedContainer.cookieStore.id;
5559
this.buildElement();
5660
this.setDisplayedContainer(displayedContainer);
5761
this.containerCloseButton.title = browser.i18n.getMessage('tooltipContainerCloseAll');
@@ -60,6 +64,7 @@ export class MenulistContainerElement extends HTMLElement {
6064
this.containerHighlightButton.title = browser.i18n.getMessage('focusToThisContainer');
6165
this.tabCount = 0;
6266
this.registerEventListeners();
67+
this.setupDragHandlers();
6368
}
6469

6570
private buildElement() {
@@ -141,8 +146,160 @@ export class MenulistContainerElement extends HTMLElement {
141146
this.containerHighlightButton.onclick = () => {
142147
this.onContainerHighlight.dispatch();
143148
};
149+
this.setupClickHandlers();
144150
}
145151

152+
private setupDragHandlers() {
153+
const containerTabsElement = this.shadowRoot?.querySelector('#container-tabs') as HTMLDivElement | null;
154+
if (!containerTabsElement) return;
155+
156+
containerTabsElement.addEventListener('dragstart', (ev: DragEvent) => {
157+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
158+
if (!tabElement || !ev.dataTransfer) return;
159+
160+
const tabId = parseInt(tabElement.getAttribute('data-tab-id') || '-1', 10);
161+
const tabIndex = parseInt(tabElement.getAttribute('data-index') || '0', 10);
162+
if (tabId === -1) return;
163+
164+
ev.dataTransfer.setData('application/json', JSON.stringify({
165+
type: 'tab',
166+
id: tabId,
167+
index: tabIndex,
168+
pinned: false,
169+
cookieStoreId: this._cookieStoreId,
170+
}));
171+
ev.dataTransfer.dropEffect = 'move';
172+
});
173+
174+
containerTabsElement.addEventListener('dragover', (ev: DragEvent) => {
175+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
176+
if (!tabElement || !ev.dataTransfer) return;
177+
178+
const json = ev.dataTransfer.getData('application/json');
179+
if (!json) return;
180+
181+
try {
182+
const data = JSON.parse(json);
183+
if ('tab' !== data.type || data.pinned) return;
184+
if (data.cookieStoreId !== this._cookieStoreId) return;
185+
ev.preventDefault();
186+
} catch (_e) {
187+
// Invalid JSON, ignore
188+
}
189+
});
190+
191+
containerTabsElement.addEventListener('drop', (ev: DragEvent) => {
192+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
193+
if (!tabElement || !ev.dataTransfer) return;
194+
195+
const targetTabId = parseInt(tabElement.getAttribute('data-tab-id') || '-1', 10);
196+
const targetIndex = parseInt(tabElement.getAttribute('data-index') || '0', 10);
197+
if (targetTabId === -1) return;
198+
199+
const json = ev.dataTransfer.getData('application/json');
200+
if (!json) return;
201+
202+
try {
203+
const data = JSON.parse(json);
204+
if ('tab' !== data.type || data.pinned) return;
205+
if (data.cookieStoreId !== this._cookieStoreId) return;
206+
ev.preventDefault();
207+
208+
browser.tabs.move(data.id, { index: targetIndex }).catch((e) => {
209+
console.error(e);
210+
});
211+
} catch (_e) {
212+
// Invalid JSON, ignore
213+
}
214+
});
215+
}
216+
217+
private setupClickHandlers(): void {
218+
const containerTabsElement = this.shadowRoot?.querySelector('#container-tabs') as HTMLDivElement | null;
219+
if (!containerTabsElement) return;
220+
221+
// Click event delegation
222+
containerTabsElement.addEventListener('click', (ev: MouseEvent) => {
223+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
224+
if (!tabElement) return;
225+
226+
const tabId = parseInt(tabElement.getAttribute('data-tab-id') || '-1', 10);
227+
if (tabId === -1) return;
228+
229+
// Get the actual clicked element from composedPath (for Shadow DOM)
230+
const path = ev.composedPath();
231+
const shadowTarget = path[0] as HTMLElement;
232+
const action = shadowTarget.getAttribute?.('data-action') || shadowTarget.closest('[data-action]')?.getAttribute('data-action');
233+
234+
if (!action) return;
235+
236+
switch (action) {
237+
case 'tab-click':
238+
// Focus the tab
239+
browser.tabs.update(tabId, { active: true }).catch((e) => {
240+
console.error('Failed to focus tab:', e);
241+
});
242+
break;
243+
244+
case 'close':
245+
// Close the tab
246+
browser.tabs.remove(tabId).catch((e) => {
247+
console.error('Failed to close tab:', e);
248+
});
249+
break;
250+
251+
case 'pin':
252+
// Toggle pin status
253+
const isPinned = tabElement.querySelector('#tab-pin-button')?.classList.contains('pinned');
254+
browser.tabs.update(tabId, { pinned: !isPinned }).catch((e) => {
255+
console.error('Failed to pin/unpin tab:', e);
256+
});
257+
break;
258+
259+
case 'set-tag':
260+
// Import ModalSetTagElement dynamically to avoid circular dependency
261+
import('./modal-set-tag').then(({ ModalSetTagElement }) => {
262+
document.body.appendChild(new ModalSetTagElement(tabId));
263+
}).catch((e) => {
264+
console.error('Failed to load ModalSetTagElement:', e);
265+
});
266+
break;
267+
}
268+
});
269+
270+
// Auxclick event (middle mouse button)
271+
containerTabsElement.addEventListener('auxclick', (ev: MouseEvent) => {
272+
if (ev.button !== 1) return; // Only handle middle click
273+
274+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
275+
if (!tabElement) return;
276+
277+
const tabId = parseInt(tabElement.getAttribute('data-tab-id') || '-1', 10);
278+
if (tabId === -1) return;
279+
280+
// Middle click closes the tab
281+
browser.tabs.remove(tabId).catch((e) => {
282+
console.error('Failed to close tab:', e);
283+
});
284+
});
285+
286+
// Contextmenu event
287+
containerTabsElement.addEventListener('contextmenu', (ev: MouseEvent) => {
288+
const tabElement = (ev.target as HTMLElement).closest('menulist-tab') as HTMLElement | null;
289+
if (!tabElement) return;
290+
291+
const tabId = parseInt(tabElement.getAttribute('data-tab-id') || '-1', 10);
292+
if (tabId === -1) return;
293+
294+
// Override context menu for Firefox
295+
browser.menus.overrideContext({
296+
context: 'tab',
297+
tabId: tabId,
298+
});
299+
}, { capture: true });
300+
}
301+
302+
146303
public setDisplayedContainer(displayedContainer: DisplayedContainer) {
147304
if (this._isPrivate) {
148305
console.assert(displayedContainer.cookieStore.userContextId == 0, "Private window should have default container only");

‎src/components/menulist-tab.ts‎

Lines changed: 7 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* eslint-disable */
12
/* -*- indent-tabs-mode: nil; tab-width: 2; -*- */
23
/* vim: set ts=2 sw=2 et ai : */
34
/**
@@ -32,11 +33,6 @@ export class MenulistTabElement extends HTMLElement {
3233
private _tabId = -1;
3334
private readonly _tabIconService = TabIconService.getInstance();
3435

35-
public readonly onTabClicked = new EventSink<number>();
36-
public readonly onPin = new EventSink<number>();
37-
public readonly onUnpin = new EventSink<number>();
38-
public readonly onClose = new EventSink<number>();
39-
4036
public constructor(tab: CompatTab, displayedContainer: DisplayedContainer) {
4137
super();
4238
this.attachShadow({ mode: "open" });
@@ -49,27 +45,6 @@ export class MenulistTabElement extends HTMLElement {
4945
this.closeButton.title = browser.i18n.getMessage('buttonTabClose');
5046
this.setTagButton.title = browser.i18n.getMessage('setTag');
5147
// this.privateIconElement.title = browser.i18n.getMessage('buttonTabPrivate');
52-
this.tabButton.onclick = () => {
53-
this.onTabClicked.dispatch(this.tabId);
54-
};
55-
this.pinButton.onclick = () => {
56-
if (this.pinned) {
57-
this.onUnpin.dispatch(this.tabId);
58-
} else {
59-
this.onPin.dispatch(this.tabId);
60-
}
61-
};
62-
this.closeButton.onclick = () => {
63-
this.onClose.dispatch(this.tabId);
64-
};
65-
this.tabButton.onauxclick = (event) => {
66-
if (event.button == 1) {
67-
this.onClose.dispatch(this.tabId);
68-
}
69-
};
70-
this.setTagButton.onclick = () => {
71-
document.body.appendChild(new ModalSetTagElement(this.tabId));
72-
};
7348
}
7449

7550
private buildElement() {
@@ -93,10 +68,12 @@ export class MenulistTabElement extends HTMLElement {
9368

9469
const tabPinButton = document.createElement('button');
9570
tabPinButton.id = 'tab-pin-button';
71+
tabPinButton.dataset.action = 'pin';
9672
tabMain.appendChild(tabPinButton);
9773

9874
const tabButton = document.createElement('button');
9975
tabButton.id = 'tab-button';
76+
tabButton.dataset.action = 'tab-click';
10077
tabMain.appendChild(tabButton);
10178

10279
const tabIcon = document.createElement('span');
@@ -109,15 +86,19 @@ export class MenulistTabElement extends HTMLElement {
10986

11087
const tabSetTagButton = document.createElement('button');
11188
tabSetTagButton.id = 'tab-set-tag-button';
89+
tabSetTagButton.dataset.action = 'set-tag';
11290
tabMain.appendChild(tabSetTagButton);
11391

11492
const tabCloseButton = document.createElement('button');
11593
tabCloseButton.id = 'tab-close-button';
94+
tabCloseButton.dataset.action = 'close';
11695
tabMain.appendChild(tabCloseButton);
11796
}
11897

11998
public setTab(tab: CompatTab) {
12099
this._tabId = tab.id;
100+
this.setAttribute('data-tab-id', tab.id.toString());
101+
this.setAttribute('data-index', tab.index.toString());
121102

122103
this.titleElement.textContent = tab.title;
123104
this.tabButton.title = tab.url;
@@ -147,13 +128,6 @@ export class MenulistTabElement extends HTMLElement {
147128
setTimeout(() => this.scrollIntoViewIfActive(), 100);
148129
}
149130

150-
// https://qiita.com/piroor/items/44ccbc2ee918bc88c3ea
151-
this.addEventListener('contextmenu', () => {
152-
browser.menus.overrideContext({
153-
context: 'tab',
154-
tabId: this._tabId,
155-
});
156-
}, { capture: true });
157131
}
158132

159133
public setDisplayedContainer(displayedContainer: DisplayedContainer) {

0 commit comments

Comments
 (0)