Ticket #4469 (closed bug: fixed)

Opened 5 years ago

Last modified 4 years ago

Accordion: newContent/oldContent misbehaving

Reported by: mikecapp Owned by:
Priority: critical Milestone: 1.8
Component: ui.accordion Version: 1.7.1
Keywords: accordion newContent oldContent Cc:
Blocking: Blocked by:

Description

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.

Attachments

scratch.html Download (1.4 KB) - added by mikecapp 5 years ago.
Demonstrator

Change History

Changed 5 years ago by mikecapp

Demonstrator

comment:1 Changed 5 years ago by scott.gonzalez

  • Milestone changed from TBD to 1.8

comment:2 Changed 5 years ago by joern.zaefferer

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

comment:3 Changed 4 years ago by 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] );
 

comment:4 Changed 4 years ago by 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

comment:5 Changed 4 years ago by 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).

comment:6 Changed 4 years ago by rdworth

  • Priority changed from major to critical

comment:7 Changed 4 years ago by scott.gonzalez

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in r3845. Thanks.

comment:8 Changed 4 years ago by ask

Thanks Scott!!

Note: See TracTickets for help on using tickets.