Bug Tracker

Opened 12 years ago

Closed 9 years ago

Last modified 8 years ago

#2604 closed bug

jQuery(html) breaks with HTML comments in Firefox 2

Reported by: scottgonzalez Owned by: scottgonzalez
Priority: low Milestone: 1.2.4
Component: core Version: 1.2.3
Keywords: html comments Cc:
Blocked by: Blocking:

Description

var html = '<div>foo</div><!-- bar -->';

// throws an error
$(html).hide();

// works fine
$('<div/>').html(html).children().hide();

Attachments (1)

clean-comments.diff (1007 bytes) - added by flesler 12 years ago.
This should fix this, but breaks 3 tests. I'll postpone this for now.. we have more critical tickets and we're close to a release.

Download all attachments as: .zip

Change History (10)

comment:1 Changed 12 years ago by davidserduke

It appears that the html comment is being created as a node in the jQuery object as expected. But when the :visible call is made on it, the jQuery.css is failing. I'd guess a check to see if the node is not appropriate in jQuery.css() would fix this.

comment:2 Changed 12 years ago by flesler

Resolution: wontfix
Status: newclosed

This seems to require checks on many places like $.filter, $.multiFilter and pretty much every DOM method. Most don't do any check, or they check for textNodes.

I'm not sure it's worthy. It's probably better to advice you to do:

$(html).filter('[nodeType=1]').hide();
or
$(html).not('[nodeType=8]').hide();

I'm closing as wontfix, reopen if you have something else to say.

comment:3 Changed 12 years ago by scott.gonzal

Resolution: wontfix
Status: closedreopened

There are already a few methods that specifically check for comment nodes to make sure they get skipped. I think the most relevant check is in jQuery.merge() which ensures that comment nodes don't get merged in. A similar check could be added to jQuery.clean() to make sure that comment nodes are stripped before calling jQuery.makeArray().

Changed 12 years ago by flesler

Attachment: clean-comments.diff added

This should fix this, but breaks 3 tests. I'll postpone this for now.. we have more critical tickets and we're close to a release.

comment:4 Changed 12 years ago by flesler

Owner: set to flesler
Status: reopenednew

comment:5 Changed 9 years ago by anonymous

hello

comment:6 Changed 9 years ago by SlexAxton

Keywords: html comments added
Owner: changed from flesler to scott.gonzalez
Priority: majorlow
Status: newpending

Is this still something anyone cares about?

comment:7 Changed 9 years ago by trac-o-bot

Status: pendingclosed

Automatically closed due to 14 days of inactivity.

comment:8 Changed 9 years ago by anonymous

This is causing problems for me. Please fix.

comment:9 Changed 9 years ago by snover

We don’t support Firefox 2 any more. This will never be fixed.

Last edited 9 years ago by snover (previous) (diff)
Note: See TracTickets for help on using tickets.