:visible/:hidden selectors unreliable #2227

shimonenator · 2015-04-21T19:31:11Z

Issue

This issue is present in both version 1 and 2 of jQuery.

It is happening on inline elements wrapping block elements, which is valid in HTML5, such as a header and an image wrapped in a single anchor.

Two issues arise:

Different browsers report offsetWidth/Height of the anchor differently. For example Firefox would report one visible anchor in the example below, and Chrome will report 0. This also highly depends on the CSS styles applied, as on the page where this issue was discovered missing images are not displayed at all instead of the common missing image icon.
While a browser may report an anchor as invisible, child elements are still visible and clickable as they inherit anchor properties.

Example

<!doctype html>
<html>
<head>
    <meta charset="utf-8">
    <script src="http://code.jquery.com/jquery-2.1.3.min.js"></script>
</head>
<body>
    <a href=#>
        <h1>Header</h1>
    </a>
    <script>
        $(function () {
            alert('Visible anchors: ' + $('a:visible').length);
        })
    </script>
</body>
</html>

Solution

There are 2 potential solutions that come to mind, but each has flaws

Loop through child elements and check each for visibility. Potentially harmful for performance, although the selector is already marked as such in the documentation.
Check parent for visibility. Probably not enough on its own, as it will report incorrectly, if anchor doesn't have visible children.

timmywil · 2015-04-21T20:43:55Z

Thank you for opening an issue. While you are correct that the behavior may be unexpected, the selector is working as documented. While the behavior differs between Firefox and Chrome, Chrome reports zero width and zero height, which technically means it is not visible according to our docs...

Visible elements have a width or height that is greater than zero.

The fact the hidden elements can have visible content can seem strange, but it's the way it is in Chrome. But more importantly, the cost of working around this would be far greater than we are willing to deal with. I don't think this is the only edge case where the :visible selector behaves in a way that can be unexpected, but I would prefer to keep the selector as simple as possible and let the edge cases be covered by other means.

dmethvin · 2015-04-21T21:00:33Z

This seems like a Chrome bug, does Safari have it as well?

mgol · 2015-04-21T21:03:18Z

This seems like a Chrome bug, does Safari have it as well?

Yes.

dmethvin · 2015-04-21T21:03:48Z

Here's a test case: http://jsfiddle.net/2tgw2yr3/

Both jsbin.com and browserstack.com are down for me at the moment.

shimonenator · 2015-04-22T01:20:11Z

@timmywil thank you for a quick reply.

First of all I'd like to apologize, I did file a duplicate issue. It seems that there's at least one earlier issue #1855 reporting this problem, which includes a solution.

Also, while it doesn't seem strange to that not visible elements can bleed visible child elements.

For the edge case degree, perhaps there should be a note in the documentation then. IMO people use jQuery to get the same results in different browsers no matter what, and they may not even realize that their selectors will sometime behave differently.

dmethvin · 2015-04-22T04:07:17Z

Funny, I thought I remembered an issue like this where getBoundingClientRect came into play but I couldn't find it because I was looking in the old tracker which was down. Maybe I'm crashing all these sites, I dunno.

The :visible and :hidden selectors aren't a good idea because they can make the browser do a lot of work to determine the answer when the layout has been changed. In many cases you can track that information in other ways and it will be a lot faster.

gibson042 · 2015-04-22T13:27:49Z

@dmethvin How old was it? @shimonenator already found #1855, and the only other case that comes to my mind is the soon-to-be-merged #2043 (which uses getClientRects to decide if .offset is meaningful). In fact, that work in particular leaves me open to a redefinition of :visible as !!(elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length).

scottgonzalez · 2015-04-22T13:32:52Z

The :visible and :hidden selectors aren't a good idea because they can make the browser do a lot of work to determine the answer when the layout has been changed. In many cases you can track that information in other ways and it will be a lot faster.

I know you love to say this as much as possible, but it's really not helpful for developers writing general purpose code, and the performance problems are irrelevant for many use cases.

timmywil · 2015-04-22T19:09:09Z

In fact, that work in particular leaves me open to a redefinition of :visible as !!(elem.offsetWidth || elem.offsetHeight || elem.getClientRects().length).

If it's that simple and getClientRects doesn't cost too much, let's do it.

shimonenator · 2015-04-22T21:07:52Z

@scottgonzalez

I think you meant :visible as

return !!((elem.offsetWidth && elem.offsetHeight) || elem.getClientRects().length)

As such, following the current implementation where :hidden is the functional method, it would be:

return (!elem.offsetWidth || !elem.offsetHeight) && !elem.getClientRects().length

Thus, it seems to me that :visible is a more performant solution, as it would only fall onto getClientRects() when either offsetWidth or offsetHeight is 0. In case with :hidden, getClientRects()will always be executed. That's for v2. It's a little more complicated in v1 due to an old Opera behavior.

shimonenator · 2015-04-22T21:12:24Z

Just discovered that there's another solution #2212 with a PR that was approved. It seems that in line with current discussion about performance, there should be some additional consideration added to #2212.

gibson042 · 2015-04-23T00:24:42Z

Upon deeper analysis, this would not be a behavior change if it makes it in for 3.0, which I strongly recommend. The git builds include 10399dd (defining :hidden by elem.offsetWidth <= 0 || elem.offsetHeight <= 0, essentially noWidth OR noHeight), but that is unreleased—all releases from 1.4 forward use logic like noWidth AND noHeight. A fix for this issue would refine the released logic and revert 10399dd.

timmywil · 2015-05-08T20:00:35Z

Actually, it looks like the proposed solution breaks the width: 0; height: 0; overflow: hidden; case, which jQuery considers hidden. http://jsbin.com/cokeku/1/edit?css,js,console,output

gibson042 · 2015-05-08T20:25:53Z

Ah, a slight behavior change, then. I'm still in favor of the getClientRects().length approach.

timmywil · 2015-05-08T20:57:08Z

In that case, what about dropping offsetWidth and offsetHeight?

gibson042 · 2015-05-08T21:03:39Z

I would make such a decision entirely on performance. My untested supposition is that offset properties are faster than getClientRects, and therefore worth including even though redundant.

timmywil · 2015-05-08T21:43:31Z

I'll do some testing.

timmywil · 2015-05-08T22:23:08Z

Ok, looks like performance necessitates keeping offsetWidth and offsetHeight. http://jsperf.com/visible-hidden-and-getclientrects

There's no difference for hidden elements, but it's about 2x slower in Chrome for visible elements to only use .getClientRects().

- Reverts behavior from 10399dd, which we never released. BR and inline elements are considered visible. - The possibility of dropping .offsetWidth and .offsetHeight was debunked by this perf: http://jsperf.com/visible-hidden-and-getclientrects Fixes jquerygh-2227

timmywil · 2015-05-08T22:32:54Z

PR

- Reverts behavior from 10399dd, which we never released. BR and inline elements are considered visible. - The possibility of dropping .offsetWidth and .offsetHeight was debunked by this perf: http://jsperf.com/visible-hidden-and-getclientrects Fixes gh-2227 Close gh-2281

shimonenator · 2015-05-14T05:41:44Z

Sorry for being late to the proposed and accepted PR, but it does not fix the issue. Please refer to my original issue description and a follow up comment.

timmywil · 2015-05-14T10:24:41Z

@shimonenator Had we wanted to keep the logic that was there, your suggestion would be right. However, the change was intentional.

FesterCluck · 2015-09-20T08:55:10Z

My apologies for commenting on a closed issue, but I believe this case warrants review.

Using ClientRectList.prototype.length an anchor tag with no text is "visible", not to mention one with child elements which have no dimension either.

Please consider https://github.com/FesterCluck/jquery/blob/master/src/css/hiddenVisibleSelectors.js

If this is not part of the new intended behavior, my apologies.

- Reverts behavior from 10399dd, which we never released. BR and inline elements are considered visible. - The possibility of dropping .offsetWidth and .offsetHeight was debunked by this perf: http://jsperf.com/visible-hidden-and-getclientrects Fixes gh-2227 Close gh-2281

timmywil closed this as completed Apr 21, 2015

timmywil reopened this Apr 22, 2015

shimonenator mentioned this issue Apr 22, 2015

use getBoundingClientRect() for the hidden filter #2212

Closed

gibson042 added CSS Behavior Change labels Apr 22, 2015

gibson042 removed the Behavior Change label Apr 23, 2015

gibson042 added this to the 3.0.0 milestone Apr 23, 2015

timmywil self-assigned this May 5, 2015

gibson042 added the Behavior Change label May 8, 2015

timmywil mentioned this issue May 8, 2015

CSS: fix :visible/:hidden selectors for inline element w/ content #2281

Closed

timmywil added the Has Pull Request label May 8, 2015

This was referenced May 8, 2015

New :visible and :hidden definitions jquery/api.jquery.com#737

Closed

Restore :visible and :hidden definitions jquery/jquery-migrate#107

Closed

timmywil closed this as completed in 79bcb29 May 12, 2015

This was referenced Sep 20, 2015

Hidden filter incorrectly includes visible elements #1855

Closed

Some elements are incorrectly excluded in Chrome marklagendijk/jquery.tabbable#3

Open

FesterCluck mentioned this issue Sep 20, 2015

:visible selector including invisible elements #2604

Closed

gibson042 mentioned this issue Oct 14, 2015

Be consistent when checking if the node is connected or not #2404

Closed

gibson042 mentioned this issue Nov 12, 2015

Not proper work of the :hidden/:visible selectors #2708

Closed

dmethvin removed the Has Pull Request label Jan 7, 2016

kevinansfield mentioned this issue Apr 20, 2016

Allow Ember to specify jQuery version TryGhost/Ghost#6741

Merged

cssmagic mentioned this issue May 18, 2016

jQuery cssmagic/ChangeLog#5

Open

jennifer-shehane mentioned this issue Jan 31, 2018

Element visibility fixes cypress-io/cypress#1242

Closed

15 tasks

lock bot locked as resolved and limited conversation to collaborators Jun 19, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

:visible/:hidden selectors unreliable #2227

:visible/:hidden selectors unreliable #2227

shimonenator commented Apr 21, 2015

timmywil commented Apr 21, 2015

dmethvin commented Apr 21, 2015

mgol commented Apr 21, 2015

dmethvin commented Apr 21, 2015

shimonenator commented Apr 22, 2015

dmethvin commented Apr 22, 2015

gibson042 commented Apr 22, 2015

scottgonzalez commented Apr 22, 2015

timmywil commented Apr 22, 2015

shimonenator commented Apr 22, 2015

shimonenator commented Apr 22, 2015

gibson042 commented Apr 23, 2015

timmywil commented May 8, 2015

gibson042 commented May 8, 2015

timmywil commented May 8, 2015

gibson042 commented May 8, 2015

timmywil commented May 8, 2015

timmywil commented May 8, 2015

timmywil commented May 8, 2015

shimonenator commented May 14, 2015

timmywil commented May 14, 2015

FesterCluck commented Sep 20, 2015

:visible/:hidden selectors unreliable #2227

:visible/:hidden selectors unreliable #2227

Comments

shimonenator commented Apr 21, 2015

Issue

Example

Solution

timmywil commented Apr 21, 2015

dmethvin commented Apr 21, 2015

mgol commented Apr 21, 2015

dmethvin commented Apr 21, 2015

shimonenator commented Apr 22, 2015

dmethvin commented Apr 22, 2015

gibson042 commented Apr 22, 2015

scottgonzalez commented Apr 22, 2015

timmywil commented Apr 22, 2015

shimonenator commented Apr 22, 2015

shimonenator commented Apr 22, 2015

gibson042 commented Apr 23, 2015

timmywil commented May 8, 2015

gibson042 commented May 8, 2015

timmywil commented May 8, 2015

gibson042 commented May 8, 2015

timmywil commented May 8, 2015

timmywil commented May 8, 2015

timmywil commented May 8, 2015

shimonenator commented May 14, 2015

timmywil commented May 14, 2015

FesterCluck commented Sep 20, 2015