Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

fix(jqLite): silently ignore after() if element has no parent #15475

Closed
wants to merge 3 commits into from
Closed

fix(jqLite): silently ignore after() if element has no parent #15475

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2191b1a
fix(jqLite): silently ignore `after()` if element has no parent
gkalpak Dec 8, 2016
2f257dc
fixup! fix(jqLite): silently ignore `after()` if element has no parent
gkalpak Dec 8, 2016
c904a05
fixup! fix(jqLite): silently ignore `after()` if element has no parent
gkalpak Dec 8, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 8 additions & 5 deletions src/jqLite.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -979,12 +979,15 @@ forEach({

after: function(element, newElement) {
var index = element, parent = element.parentNode;
newElement = new JQLite(newElement);

for (var i = 0, ii = newElement.length; i < ii; i++) {
var node = newElement[i];
parent.insertBefore(node, index.nextSibling);
index = node;
if (parent) {
newElement = new JQLite(newElement);

for (var i = 0, ii = newElement.length; i < ii; i++) {
var node = newElement[i];
parent.insertBefore(node, index.nextSibling);
index = node;
}
}
},

Expand Down
8 changes: 8 additions & 0 deletions test/jqLiteSpec.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2182,6 +2182,14 @@ describe('jqLite', function() {
span.after('abc');
expect(root.html().toLowerCase()).toEqual('<span></span>abc');
});


it('should not throw when the element has no parent', function() {
var span = jqLite('<span></span>');
expect(function() { span.after('abc'); }).not.toThrow();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add an expect that the span is still the span?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The test is now failing :o is it because toJqEqual is wrapped in jqlite?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😱 It passed locally (on Chrome). IEs seems to have a hard time. I'll take a look.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking DOM Nodes for deep equality isn't an intended usecase apparenty 😃
I changed the expectations.

expect(span.length).toBe(1);
expect(span[0].outerHTML).toBe('<span></span>');
});
});


Expand Down