Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.innerWidth(), .innerHeight() mishandling scroll bars in 3.2.x #3589

Closed
hashchange opened this issue Mar 23, 2017 · 4 comments
Closed

.innerWidth(), .innerHeight() mishandling scroll bars in 3.2.x #3589

hashchange opened this issue Mar 23, 2017 · 4 comments
Assignees
Labels
Milestone

Comments

@hashchange
Copy link

@hashchange hashchange commented Mar 23, 2017

.innerWidth() and .innerHeight() no longer return consistent results when scroll bars appear on an element.

Previously, the methods returned the size of the box up to the border, whether or not scroll bars are present. (Scroll bars are rendered inside of the border.) That's how the methods behaved from jQuery 1.2.6, when they were introduced, up to jQuery 3.1.1.

Starting with 3.2.0, the area covered by scroll bars is subtracted from the return value in Chrome and Opera (ie, Blink), IE, and Edge. That happens when the box-sizing of the element is set to content-box.

See the test case at JS Bin.

FF is not affected by the bug. Blink and Edge behave correctly when box-sizing is set to border-box. In IE, the return value is still broken then.

The issue doesn't exist in Safari and mobile browsers because they render scroll bars as temporary overlays which don't take up space in the element.

It seems that the bug is a direct result of the fix #3561 for #3193. In getWidthOrHeight, you have switched to computed values for establishing the dimensions. You had used getBoundingClientRect() previously, and offsetWidth/offsetHeight before that.

I totally appreciate the difficulties raised in #3193. Still, the new approach backfires with regard to scroll bars. FF places them inside of the padding edge, while Blink and Edge place them inside of the inner border edge, but outside of the padding edge.

That difference between browsers doesn't matter when gBCR or offsetWidth/Height are queried. It also doesn't matter for the computed values of width and height as long as they are based on the border box (well, except in IE, for whatever reason). But it does change the computed values for the size of the content box, in the affected browsers.

@timmywil
Copy link
Member

@timmywil timmywil commented Mar 27, 2017

We're not sure the best course here. Why do browsers differ? Is the spec ambiguous about including scrollbars in computed css?

@gibson042
Copy link
Member

@gibson042 gibson042 commented Mar 27, 2017

All browsers correctly render the scrollbars between border and padding, breaking our naïve assumptions that margins wrap borders, which wrap padding, which wraps content. And as can be seen in the Developer Tools inspector, they disagree about how to do it for box-sizing: content-box—Chrome and Edge account for them by reducing the computed content area (and hence also .innerWidth()/.innerHeight()), while Firefox does not, essentially carving out part of the content box and moving it past the padding without reducing dimensions. For box-sizing: border-box, they all use the Firefox model.

However, it seems like the Firefox model is always incorrect with respect to the CSS basic box model draft: "the containing block is the content edge of the parent’s principal box… any space taken up by the scrollbars should be excluded from (subtracted from the dimensions of) any containing block". And this really feels like a spec bug—it should be fairly easy to get the space consumed by non-floating scrollbars.

But since it isn't, we can include scrollbars in .innerWidth() by adding in the difference of a border-box: content-box element's offsetWidth and the sum of its computed width plus padding plus border width (e.g., paddingAreaWidth = width + paddingWidth + (offsetWidth - cssWidth - paddingWidth - borderWidth), and likewise for .innerHeight().

@gibson042
Copy link
Member

@gibson042 gibson042 commented Apr 17, 2017

For more clarity: augmentWidthOrHeight should consider scrollbar gutters to be part of padding on lines 105 and 116.

gibson042 added a commit to gibson042/jquery that referenced this issue May 4, 2017
gibson042 added a commit to gibson042/jquery that referenced this issue May 4, 2017
gibson042 added a commit that referenced this issue Jun 19, 2017
gibson042 added a commit to gibson042/jquery that referenced this issue Jul 18, 2017
gibson042 added a commit that referenced this issue Jul 18, 2017
@lock lock bot locked as resolved and limited conversation to collaborators Jun 17, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.