Bug Tracker

Opened 12 years ago

Closed 11 years ago

Last modified 11 years ago

#3171 closed bug (invalid)

wrap() only adds sibling in some instances

Reported by: logicmason Owned by:
Priority: major Milestone: 1.3
Component: core Version: 1.2.6
Keywords: wrap() adds sibling Cc: logicmason, flesler, Julian-Birch
Blocked by: Blocking:

Description

I have found that wrap() is not working properly when trying to wrap with elements containing text. For example, taking this HTML: 

...
    <ul>
        <li><a target="..." href="...">Text1</a></li>
        <li><a target="..." href="...">Text2</a></li>
    </ul>
...

If I attempt to wrap the <a> element around the <li> element using:

(NOTE: $(this) in all the examples shown on this page relate to the <li> elements) 

var linkElem = $(this).find( "a:first" );
		
$(this).wrap( linkElem );
$(this).remove( "a" );

The result (seen using firebug) is that the inner <a> element is added as a sibling to the wrap() caller element: 

...
    <ul>
        <a target="..." href="...">Text1</a>
        <li><a target="..." href="...">Text1</a></li>
        <li><a target="..." href="...">Text2</a></li>
    </ul>
...

I note as well the call to remove() did not remove the inner element. To workaround this, it seems you actually have to remove the wrapping element's text, then wrap it around the inner element, then re-apply the text to the inner element: 

var linkElem = $(this).find( "a:first" );
var linkText = $(linkElem).text();
		
$(linkElem).empty();
$(this).wrap( linkElem );
$(this).empty();
$(this).text( linkText );

created: 

...
    <ul>
        <a target="..." href="..."><li>Text1</li></a>
        <a target="..." href="..."><li>Text2</li></a>
    </ul>
...

Have only tried this in FF3.

Change History (5)

comment:1 Changed 12 years ago by flesler

need: Review → Test Case

Can you make a small test case out of this ? An html file with the js needed for this error to come up.

comment:2 Changed 11 years ago by Julian-Birch

This looks like the same bug I'm experiencing. The following demonstrates it pretty nicely. Happens (amazingly) portably on a wide range of browsers.

Makes certain things pretty impossible...

Julian.

<html> <head> <script type="text/javascript" src="/jquery-1.2.6.js"></script> <script type="text/javascript"> $(document).ready(function() { elements = $("div > *"); elements.wrap("span"); }); </script> </head> <body> <form><div class='Box'> <label for='repeat'>Repeat</label> <span> <input id='repeat' type='text' value='Box1' /> </span> </div> </form> </body> </html>

comment:3 Changed 11 years ago by flesler

Cc: logicmason flesler Julian-Birch added
need: Test Case → Review

Ok, will check the test case asap (not today).

comment:4 Changed 11 years ago by dmethvin

Resolution: → invalid
Status: new → closed

The Julian-Birch test case isn't valid: 

elements.wrap("span");

works fine with this: 

elements.wrap("<span></span>");

On the OP's example, wrap is not intended to deal with cases where there are already sibling elements inside the wrapping element, which is the case with <a href="">text</a>. The text is a child of the <a>; using it as the wrapping element does not cause the <li> to adopt the text.

Note that <a> is an inline element and <li> is a block element, so the HTML created is invalid. However, your final approach would be the right one to use to get the HTML you wanted.

comment:5 Changed 11 years ago by Julian-Birch

Agreed. Sorry, I managed to find the problem with the test case, but couldn't track down the ticket earlier. Everything works as per spec.

Note: See TracTickets for help on using tickets.