Skip to main content

Search and Top Navigation

#6058 closed bug (fixed)

Opened September 16, 2010 02:31PM UTC

Closed December 04, 2012 12:22AM UTC

Dialog overlays are not properly reused when multiple instances of a Dialog exist

Reported by: emeerson Owned by: emeerson
Priority: minor Milestone: 1.10.0
Component: ui.dialog Version: 1.8.2
Keywords: overlay Cc: rbudigelli@ngpsoftware.com, emeerson@ngpsoftware.com
Blocked by: Blocking:
Description

When multiple dialog instances exist at the same time (but one may be hidden from view), the internal overlay oldInstances stack attempts to reuse a previous overlay but does not make the overlay properly visible.

Removing oldInstances.pop() from line 6143 alleviates this issue. Since it is implemented for addressing a browser-specific deficiency (memory-leak, no less), should this be implemented for all browser(s) & versions (while I understand the mantra of feature checking, not browser checking, this is not an issue of Browser standards or feature support, but a straight-up memory leak in IE).

Attachments (0)
Change History (12)

Changed October 01, 2010 06:51PM UTC by scottgonzalez comment:1

component: ui.coreui.dialog

Changed September 07, 2011 12:50PM UTC by scottgonzalez comment:2

cc: rbudigelli@ngpsoftware.com; emeerson@ngpsoftware.comrbudigelli@ngpsoftware.com, emeerson@ngpsoftware.com
keywords: → overlay

Changed October 11, 2012 09:02PM UTC by scottgonzalez comment:3

milestone: TBD1.10.0

Changed October 16, 2012 01:55PM UTC by bavanyo comment:4

resolution: → duplicate
status: newclosed

Duplicate of #5901 .Seems to be a duplicate/related issue to #5901

Changed October 16, 2012 03:10PM UTC by scottgonzalez comment:5

resolution: duplicate
status: closedreopened

I don't think these are related. One is a feature request and one is a memory leak bug.

Changed October 27, 2012 08:19PM UTC by mikesherov comment:6

status: reopenedopen

Changed November 30, 2012 12:20PM UTC by jzaefferer comment:7

owner: → emeerson
status: openpending

Needs to be reproduced, state should be "new", but can't set that since it was reopened. I don't know what "(but one may be hidden from view)" means here - closed dialog, manually hidden?

Changed December 01, 2012 01:00AM UTC by scottgonzalez comment:8

The reuse was added to address the memory leak in IE 6/7/8 reported in #5185. I'm wondering if we should just get rid of the reuse now that we're using fixed positioning for overlays, so they should be much smaller and therefore use less memory.

Changed December 03, 2012 06:09PM UTC by jzaefferer comment:9

It the memory usage is still a problem, which we'd need to verify, we could remove the opacity from the overlay, for IE<=8.

Changed December 03, 2012 06:30PM UTC by scottgonzalez comment:10

It seems like people are willing to deal with some level of memory leak from this. I'd actually be fine with just getting rid of the workaround, leaving the opacity in the theme and telling people to override the styles if they care about the leak.

Changed December 03, 2012 11:36PM UTC by jzaefferer comment:11

status: pendingopen

Now would be a good time to challenge a few assumptions. Removing the workaround will surely address this issue, whatever it actually is.

Changed December 04, 2012 12:22AM UTC by Jörn Zaefferer comment:12

resolution: → fixed
status: openclosed

Dialog: Remove the instance-storing for the overlay, just create one whenever it is needed. Heavily simplifies the code, while the memorly leak should be hardly an issue anymore, since fixed positioning restricts the overlay size to the window dimensions. Fixes #6058 - Dialog overlays are not properly reused when multiple instances of a Dialog exist.

Changeset: 1e8baf568365f8edc833439315f76e5efe1ba9b6