Bug Tracker

Modify

Ticket #3552 (closed bug: fixed)

Opened 5 years ago

Last modified 4 years ago

.wrapInner() won't work on empty elements

Reported by: dtetto Owned by: flesler
Priority: minor Milestone: 1.4
Component: core Version: 1.4a2
Keywords: wrapInner Cc:
Blocking: Blocked by:

Description

$(elem).wrapInner('<div/>') does nothing if the original element is empty.

There is some logical elbow-room here -- one could argue that the wrap shouldn't be performed if there's nothing to wrap -- but the current behavior feels like a bug to me.

Based on the syntax, the transformation is being performed on the original element, not on its children: $(elem).wrapInner('<div/>'); // wrap the inside of this element with a div vs. $(elem).contents().wrapAll('<div/>'); // wrap this elements contents (if any) with a div

It's easy to get around this:

$.fn._wrapInner = $.fn.wrapInner;

$.fn.wrapInner = function(wrapper){

if(this.contents().length) return this._wrapInner(wrapper);

else return this.append(wrapper);

}

But I'd argue it ought to be changed in the library itself unless there's reason to believe a lot of folks are relying on the current behavior.

Attachments

wrapInner.testcase.html Download (1.3 KB) - added by Marc Diethelm 5 years ago.
Testcase
core.js.r6267.diff Download (390 bytes) - added by Marc Diethelm 5 years ago.
diff: fixes bug against r6267 (core.js)
wrapInner.diff Download (566 bytes) - added by ricardobeat 5 years ago.
alternative patch (core.js r6270)

Change History

Changed 5 years ago by Marc Diethelm

Testcase

comment:1 Changed 5 years ago by Marc Diethelm

IMO wrapInner should behave consistently whether there are actual nodes to enclose or not.

I'm attaching a simple testcase (uses  http://code.jquery.com/nightlies/jquery-nightly.js).

comment:2 Changed 5 years ago by Marc Diethelm

And here's a "patch" (a diff actually) that fixes the problem. I hope it's ok style wise...

Changed 5 years ago by Marc Diethelm

diff: fixes bug against r6267 (core.js)

Changed 5 years ago by ricardobeat

alternative patch (core.js r6270)

comment:3 Changed 5 years ago by ricardobeat

Added an alternative patch with a simpler/faster check for .firstChild and append instead of html (faster). Hope it's useful.

comment:4 Changed 5 years ago by dmethvin

  • Component changed from unfilled to core

comment:5 Changed 4 years ago by john

  • Status changed from new to closed
  • Version changed from 1.2.6 to 1.4a2
  • Resolution set to fixed
  • Milestone changed from 1.3 to 1.4

Please follow the  bug reporting guidlines and use  jsFiddle when providing test cases and demonstrations instead of pasting the code in the ticket.

View

Add a comment

Modify Ticket

Action
as closed
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.