Bug Tracker

Opened 8 years ago

Closed 8 years ago

Last modified 8 years ago

#10591 closed bug (invalid)

Sizzle filter should skip display:none elements

Reported by: jabouillei@… Owned by:
Priority: low Milestone: None
Component: selector Version: 1.7rc1
Keywords: Cc:
Blocked by: Blocking:

Description

For example, the ":even" filter should ignore elements with display=none because elements with display=none should be treated as if they don't exist. (See text below copy and pasted from w3.org.) When skipping, the index used for filtering should not be incremented.

I've provided a complete example here: http://jsfiddle.net/3NZ8C/

Here is the quote from w3.org regarding disp:

This value causes an element to not appear in the formatting structure (i.e., in visual media the element generates no boxes and has no effect on layout). Descendant elements do not generate any boxes either; the element and its content are removed from the formatting structure entirely. This behavior cannot be overridden by setting the 'display' property on the descendants.

Please note that a display of 'none' does not create an invisible box; it creates no box at all. CSS includes mechanisms that enable an element to generate boxes in the formatting structure that affect formatting but are not visible themselves. Please consult the section on visibility for details.

The fix might be easy. (Maybe I've overlooked something.) I just use an i_among_displayed variable and skip cases of display===none:

if ( match ) {

var i_among_displayed = 0; for ( i = 0; (item = curLoop[i]) != null; i++ ) {

if ( item ) {

if ( jQuery.css( item, "display" ) === "none" ) {

continue;

} found = filter( item, match, i_among_displayed, curLoop );

...

i_among_displayed++;

I just made this change in a fork in github. After I click "Create ticket", I'll submit a pull request referencing this ticket.

Change History (3)

comment:1 Changed 8 years ago by timmywil

Component: unfiledselector
Priority: undecidedlow
Resolution: invalid
Status: newclosed

This is expected behavior. Even though they are not painted, display:none elements should still be selected. In other words, display has no effect on selections:

http://www.w3.org/TR/css3-selectors/#nth-child-pseudo http://jsfiddle.net/timmywil/N7GzZ/

comment:2 Changed 8 years ago by dmethvin

Why not just use :visible:even instead? http://jsfiddle.net/3NZ8C/1/

As far as consistency goes, :nth-child(even) doesn't exhibit this "visible elements only" behavior so it doesn't seem like a good idea to change :even.

I *would*, however, like to see a standard W3C selector for visibility so then you say something like :visible:nth-of-type(even).

comment:3 Changed 8 years ago by jabouillei@…

I'm sure you guys deal with this stuff much more than I do, so I'll have to take your word for it. It won't hurt me because I can use :visible:even, but even after re-reading the spec, it still sounds to me like "display:none"s should be skipped for all these selectors. Granted, that's based on informative rather than normative text. The w3 page timmywil referenced states: "For example, this allows the selectors to address every other row in a table, and could be used to alternate the color of paragraph text in a cycle of four." and "tr:nth-child(2n+1) /* represents every odd row of an HTML table */" When I combine those examples with w3 statement that I originally quoted: "Please note that a display of 'none' does not create an invisible box; it creates no box at all." it leaves me with the impression the "display:none"s should be skipped.

So... there's my $.02 for the record. At least maybe this ticket will come up if someone does a search. (I did a search because I suspected that this would have been fixed by now unless someone disagreed. I didn't get any hits.)

Thanks for considering it!

Note: See TracTickets for help on using tickets.