Community
Participate
Working Groups
Using HEAD as of 05/28 1. Go to the Navigator. 2. Open the browser's DOM Inspector 3. Without selecting any items, repeatedly click on the "Actions" menu. 4. Every time the "Click on an item..." popup is displayed, an element like this is injected into the DOM: <div class="dijitPopup dijitTooltipDialogPopup" style="z-index: 1000; top: 94px; left: 389px; visibility: visible; display: none; " role="presentation" id="popup_193" dijitpopupparent=""><div role="presentation" tabindex="-1" class="dijitTooltipDialog dijitTooltipABLeft dijitTooltipBelow" title="" id="dijit_TooltipDialog_78" widgetid="dijit_TooltipDialog_78" style="top: 0px; visibility: visible; "> ... These elements never seem to be cleaned up. A similar problem occurs when you change the selection and open the "Actions" menu. This causes new drop-down menu elements to be added to the DOM every time.
I am not sure if it is an ideal solution but in compare widget case when I need to destroy the old one and replace by a new one I always assign a unique Id for the dijit and use dijit.destroyRecursively() to release it. Using Id is not mandatory but can warn you when you try to create it before releasing the old one.
The first problem (the "make a selection tooltip dialog) is fixed in http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=88fe90f13c7721492963f3dd263858bc4a9f3bfb I had another case (parameter collection tooltip dialogs) with the same problem. I was closing but not destroying. Easy to fix in the framework. The second problem requires more client knowledge than I'd like. This is where clients are rerendering the toolbar when the selection changes. Up to now, I've never had the command framework be responsible for emptying anything when it renders commands. The idea was that someone might want to render commands of different scopes/handlers/items in the same dom element. I think it should still be a client responsibility to empty their command dom area, but the problem is we are using only empty and thus not destroying related dijit widgets. Libing's grid stuff was the first place where a consumer of the command framework had to know there were dijits involved and treat them specially. I was hoping not to propagate this knowledge up to "normal consumers" but something needs to happen here, even if it's just the client telling the framework..."Clean up"
I think the solution here is to define a commandService.empty() which could do any cleanup of dijit items, and any other caches that its keeping regarding rendered commands. However this is going to touch nearly every page and we are trying to get a good build for RC1 within hours, so I will work on this early in RC2.
I have a fix for this. Long run we need to change every page to call commandService.destroy(toolbar) instead of dojo.empty(toolbar) But the reality is that the only page which has menus in its toolbars is the navigator, so we could fix the leak by making the change only in fileCommands.js.... tempting for RC1.
Opened bug 382126 for the long term fix to every page. We'll do the most local fix for RC2 (not a blocker for RC1 so I'm chilling out...)
Mark, could you fetch remote and cherry pick this commit? 881df523665e16d3998438fce3cd30d316441b0f This fixes the leak with a new method in commands.js but we are only calling it for now in the navigator. After 0.5 I'll change all command clients to use this method (in bug 382126). You can push this fix if you approve.
+1, pushed http://git.eclipse.org/c/orion/org.eclipse.orion.client.git/commit/?id=b67e1fc30ceedc966b66f3f39304164c282a2be5
see comment 7
