Accordion: newContent/oldContent misbehaving

[mikecapp]

In the handler for an accordionchange event, the ui object exposes oldHeader/newHeader and oldContent/newContent. Given the markup

<div id="accordion">
 <div>
  <a href="#">First header</a>
  <div><p>This is a P wrapped in a DIV</p></div>
 </div>
 <div>
  <a href="#">Second header</a>
  <div><p>This is a P wrapped in a DIV</p></div>
 </div>
</div>

the headers are what you'd expect - the <a> elements. The oldContent/newContent properties, on the other hand, return not the <div>s following the headers as you'd expect, but the <p> elements nested inside those divs. Without the nested <p>s, oldContent/newContent are empty.

This is surprising, annoying, inconsistent and not obviously useful.

[mikecapp]

Demonstrator

[Jörn Zaefferer]

Valid ticket, oldContent/newContent needs fixing, most likely a regression.

[ask]

This is the change that introduced the bug as far as I can tell.

Ask

Author: paul.bakaus <paul.bakaus@a1defee3-d24d-0410-b1e3-3171fe540af7>
Date:   Sun Jan 25 21:20:15 2009 +0000

    accordion: fix for newContent/oldContent in the UI object that was pointing to the workaround wrapper (temporary, until we remove the wrapper completely)
    
    git-svn-id: http://jquery-ui.googlecode.com/svn/trunk@1794 a1defee3-d24d-0410-b1e3-3171fe540af7

diff --git a/ui/ui.accordion.js b/ui/ui.accordion.js
index 95fd55f..0335f22 100644
--- a/ui/ui.accordion.js
+++ b/ui/ui.accordion.js
@@ -251,8 +251,8 @@ $.widget("ui.accordion", {
                                options: o,
                                newHeader: clickedIsActive && !o.alwaysOpen ? $([]) : clicked,
                                oldHeader: this.active,
-                               newContent: clickedIsActive && !o.alwaysOpen ? $([]) : toShow,
-                               oldContent: toHide
+                               newContent: clickedIsActive && !o.alwaysOpen ? $([]) : toShow.find('> *'),
+                               oldContent: toHide.find('> *')
                        },
                        down = this.headers.index( this.active[0] ) > this.headers.index( clicked[0] );
 

[ask]

Subject: [PATCH] Accordion: Fix ui.newContent/ui.oldContent (#4469)

---
 ui/jquery.ui.accordion.js |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui/jquery.ui.accordion.js b/ui/jquery.ui.accordion.js
index ab775ca..d1a7abd 100644
--- a/ui/jquery.ui.accordion.js
+++ b/ui/jquery.ui.accordion.js
@@ -313,8 +313,8 @@ $.widget("ui.accordion", {
 				options: o,
 				newHeader: clickedIsActive && o.collapsible ? $([]) : clicked,
 				oldHeader: this.active,
-				newContent: clickedIsActive && o.collapsible ? $([]) : toShow.find('> *'),
-				oldContent: toHide.find('> *')
+				newContent: clickedIsActive && o.collapsible ? $([]) : toShow,
+				oldContent: toHide
 			},
 			down = this.headers.index( this.active[0] ) > this.headers.index( clicked[0] );
 
-- 
1.7.0

[ask]

(Specifically it was r1878 ("Accordion: Fixed #3788: Removed wrapper divs and smoothed animations, allowing accordions to work with dl's and ul's again.") that broke this).

[Scott González]

Fixed in r3845. Thanks.

[ask]

Thanks Scott!!