#8119 closed bug (fixed)
Dialog: Destroying a dialog leaves some styles changed
Reported by: | Domenic | Owned by: | |
---|---|---|---|
Priority: | minor | Milestone: | 1.10.0 |
Component: | ui.dialog | Version: | 1.8.17 |
Keywords: | Cc: | ||
Blocked by: | Blocking: |
Description
When you do $(el).dialog("destroy")
, el
has leftover dialog-related style
, scrolltop
, and scrollleft
attributes.
The style attributes in particular are troubling, since they override any non-inline styles the original element might have. But all three attributes overwrite previously-existing values.
Here is a test case to show what I mean. The bug is present in all browsers, but the test case only works in browsers with outerHTML
support, so in particular not Firefox before 11.
If someone could quickly agree with me that this is a bug, then I will work on a pull request to fix it.
Change History (10)
comment:1 Changed 10 years ago by
Milestone: | 1.9.0 → 1.10.0 |
---|
comment:2 Changed 10 years ago by
Status: | new → open |
---|---|
Summary: | Destroying a dialog leaves style, scrollleft, and scrolltop leftovers → Dialog: Destroying a dialog leaves style, scrollleft, and scrolltop leftovers |
comment:4 Changed 10 years ago by
Replying to joern.zaefferer:
Since we aren't (and probably can't) including the style propert in domEqual, how can or should we test this?
We should try again. I don't remember what failures we had when I added style checks, but I think there were very few issues.
Currently dialog makes the assumption that the dialog element is hidden when the dialog is created and calls show(), then hide() on destroy, leaving display:none in the DOM. Is this assumption still correct?
Well, the element will generally be hidden, but it may not be explicitly hidden with display: none;
on the element.
comment:5 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Dialog: Cleanup style properties on _destroy. Reenables style check in domEqual, while removing commented and unnecessary old code. Fixes #8119 - Dialog: Destroying a dialog leaves style, scrollleft, and scrolltop leftovers.
Changeset: d687a1b10d1e176a6a8dd4420f1ea3a890640da0
comment:6 Changed 10 years ago by
Revert "Dialog: Cleanup style properties on _destroy. Reenables style check in domEqual, while removing commented and unnecessary old code. Fixes #8119 - Dialog: Destroying a dialog leaves style, scrollleft, and scrolltop leftovers."
This reverts commit d687a1b10d1e176a6a8dd4420f1ea3a890640da0.
Changeset: 050e71bdd88708ce6e8462a89af4399cffa72cf3
comment:7 Changed 10 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
comment:8 Changed 10 years ago by
Status: | reopened → open |
---|---|
Summary: | Dialog: Destroying a dialog leaves style, scrollleft, and scrolltop leftovers → Dialog: Destroying a dialog leaves some styles changed |
I think it's unreasonable to expect the scroll position to be set back.
comment:9 Changed 10 years ago by
Resolution: | → fixed |
---|---|
Status: | open → closed |
Dialog: Remove width, min-height, height styles on destroy. Fixes #8119 - Dialog: Destroying a dialog leaves some styles changed.
Changeset: 3c2acc322782cc08e575405f8123029342e33542
comment:10 Changed 10 years ago by
Dialog: Restore inline styles for dimensions/display. Fixes #8119 - Dialog: Destroying a dialog leaves some styles changed.
Changeset: f59f5a8b12d50c87ba6e2fe47a1804a23535b3cf
Since we aren't (and probably can't) including the style propert in domEqual, how can or should we test this?
Adding this to _destroy removes a few of the style properties we add:
Currently dialog makes the assumption that the dialog element is hidden when the dialog is created and calls show(), then hide() on destroy, leaving display:none in the DOM. Is this assumption still correct?