Bug Tracker

Opened 7 years ago

Closed 6 years ago

#3552 closed bug (fixed)

.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:
Blocked by: Blocking:

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 (3)

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

Download all attachments as: .zip

Change History (8)

Changed 7 years ago by Marc Diethelm

Testcase

comment:1 Changed 7 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 7 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 7 years ago by Marc Diethelm

diff: fixes bug against r6267 (core.js)

Changed 7 years ago by ricardobeat

alternative patch (core.js r6270)

comment:3 Changed 7 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 6 years ago by dmethvin

  • Component changed from unfilled to core

comment:5 Changed 6 years ago by john

  • Milestone changed from 1.3 to 1.4
  • Resolution set to fixed
  • Status changed from new to closed
  • Version changed from 1.2.6 to 1.4a2
Note: See TracTickets for help on using tickets.