The conflation of unrelated behaviors is Shadow DOM support and applicability of getBoundingClientRect to disconnected elements. As I mentioned in #1784, I'd like to see this entire block (and all detection or special treatment of disconnection) removed.

I'd rather just return; here... { top: 0, left: 0 } looks like a real value, when we know it cannot be.

Sounds resonable. Will change it in next commit.

This is overkill; the only properties that matter for detecting visibility are width and height—and note that there is literally no way to differentiate no-content inline elements that happen to be at viewport origin from disconnected or display:none elements with gBCR, since they all have zero width/height/top/left. All are arguably "hidden" in the parlance of our API documentation, so our handling comes down to a matter of taste (which is responsible for the churn here; sorry about that @NekR):

• assume any data is valid and always apply scroll offset
  • inaccurate for disconnected/display:none
• assume zero width and zero height indicate disconnected/display:none; return undefined or null or { top: 0, left: 0 }
  • inaccurate for rare zero-size edge cases
• consider zero width or zero height "hidden"; return undefined or null or { top: 0, left: 0 }
  • inaccurate for slightly less rare zero-size edge cases

All the "inaccurate" results are allowed by our documentation, but are probably not equally useful to consumers (and in fact, the undefined or null results may be beneficial). Option 2 is only relevant if we want to give normal treatment to single-dimension elements (e.g., explicitly-sized elements or no-content display:block), so what's more valuable: accurate position for zero-size elements (first option), or distinct output for probably-unpositionable input (third or second option, especially with non-rect return)?

@dmethvin, @mzgol, @timmywil: care to weigh in?

Edit: I personally lean towards the second option and null/undefined, which tries hard to give a position, but not so hard that one is mistakenly applied to truly unpositioned elements.

In my personal opinion, I do not think what visually hidden element is always hidden by perspective of developer or even browser. If element participates in browser layout -- then that element absolutely not hidden. Consider following situation:

Someone uses empty <div> like a placeholder for future content (ajax loading for example). By some situation -- user scroll page so that position <div> becomes in view (or other same situation). And then developer needs to load content in that <div>.
So, to determine this situation developer absolutely need to know position of that <div>, even if height of that <div> is 0.

Try to run this example line by line in console ($0 === some block in this page):

var a = document.createElement('div');
$0.appendChild(a);
a.getBoundingClientRect();
// ClientRect {height: 0, width: 653.6363525390625, left: 124.90056610107422, bottom: 405.553955078125, right: 778.5369262695312…}
a.style.margin = '20px';
a.getBoundingClientRect();
// ClientRect {height: 0, width: 613.6363525390625, left: 144.9005584716797, bottom: 425.553955078125, right: 758.5369262695312…}

As you can see, practically that element is not hidden (even if it's not visible by eye on a page). Even more, it still participates in layout and when you will add margin to element, it still will be hidden, but will take 40px height on a screen.

If this is "rare" case, then okay, I will simplify that condition. But for now I will wait for other responses.

I agree with you, which is why I'm advocating a test like !rect.width && !rect.height that will not trip up on single-dimension elements such as your no-content div. But at the same time, we must recognize that dimensionless (e.g., no-content inline) can be disconnected/display:none false positives (or vice versa)... I don't think the extra top/left checks are worth it, given that even they are imperfect.

I don't think the extra top/left checks are worth it

Do you talk here about performance?

There is only one possible false-positive situation when element is placed at top-left corner and screen is not scrolled (or simply position: fixed).
If you really care about this situation we can use getClientRects() which returns 0 length list for elements which does not participate in layout, according to spec:

1. If the element on which it was invoked does not have an associated layout box return an empty DOMRectList object and stop this algorithm.

http://www.w3.org/TR/cssom-view/#dom-element-getclientrects

So this might be the case:

if ( !element.getClientRects().length ) {
  // We are not interested
}

Also, if you do not like getClientRects() method, we can use something like .offsetParent which returns null almost in same situations (but getClientRects() is better of course because .offsetParent will require a lot of other checks):

The offsetParent attribute must return the result of running these steps:

1. If any of the following holds true return null and terminate this algorithm:

• The element does not have an associated CSS layout box.
• The element is the root element.
• The element is the HTML body element.
• The element’s computed value of the position property is fixed.

http://dev.w3.org/csswg/cssom-view/#dom-htmlelement-offsetparent

Anyway, we can be here accurate as much as we want.

I lean towards 2 as well.

I don't think the extra top/left checks are worth it

Do you talk here about performance?

No; code size.

If you really care about this situation we can use getClientRects()

Oh perfect, I forgot that was exposed directly!

// Return a non-empty value only if it the element has layout
if ( rect.width || rect.height || elem.getClientRects().length ) {
    return {
        top: rect.top + win.pageYOffset - docElem.clientTop,
        left: rect.left + win.pageXOffset - docElem.clientLeft
    };
}

And just fall out of the function with an implicit undefined otherwise.

👍

@gibson042 we do not need first check about rect width/height if we going to use getClientRects() because getBoundingClientRect() is backed by it, so in your example check for rect width/height is overkill because in all cases there they will match, getClientRects() will also match too.

So, you guys want to match only truly hidden/disconnected elements (with CSS layout) then it's enought to check for getClientRects() length. If you plan to support "Option 2" where we treat zeros of width & height as hidden element -- then check only for those properties is enough.

But I also see reason in your check, @gibson042. In perform world calling of getClientRects() and then getBoundingClientRect() should trigget only one layout (or zero), but I am not really sure about all browsers. So in this case your check seems resonable -- call getClientRects() only when it's required.

Anyway, I am ready to use each of those options, so please, pick one and I will be happy to update my PR (yes I saw what @dmethvin and @timmywil prefer "Option 2", but it's still not clear to me which we are really gonna use).

For some reason our API users seem to strongly prefer silent nonsense results without errors for edge cases, so returning an object there might result in fewer bug reports.

We have three internal uses of .offset() as well; one in the offset setter (see above) and two in .position(). I'm willing to let the latter throw on no-layout input, but I'm not sure about the former—is there a case for essentially letting the offset setter proxy .css({ top: y, left: x }) in the absence of a layout box? If not, the code changes there go away and that throws as well. And having typed it, I'm thinking that throwing there too is reasonable.

match only truly hidden/disconnected elements (with CSS layout)

call getClientRects() only when it's required

Yes to both.

Yes to both.

Got it now, very good idea @gibson042.

Will make all these changes soon.

This relates to #2043 (comment) ... do we want to suppress exceptions when setting document-relative offset on no-layout elements? I'd like some other opinions here, but personally am inclined to drop this change and let the call throw.

Ditto. +1 for throwing.

Would need to mention it in the release notes

This relates to #2043 (comment) ... do we want to suppress exceptions when setting document-relative offset on no-layout elements? I'd like some other opinions here, but personally am inclined to drop this change and let the call throw.

Do you suggest to just remove || { top: 0, left: 0 } or guard for curOffset and throw manually custom error?

Per meeting, remove || { top: 0, left: 0 }.

Actually, after further discussion, we'll leave as is and make this change separately. Sorry to confuse! Nothing further for you to do here @NekR

Tracking in #2114.