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

css() reports wrong width/height in Chrome on certain zoom levels #3808

Closed
bschwarzent opened this issue Oct 4, 2017 · 12 comments
Closed

css() reports wrong width/height in Chrome on certain zoom levels #3808

bschwarzent opened this issue Oct 4, 2017 · 12 comments
Labels
Milestone

Comments

@bschwarzent
Copy link

@bschwarzent bschwarzent commented Oct 4, 2017

jQuery 3.2.1 reports wrong sizes for elements with box-sizing: border-box in Chrome (v61) on certain browser zoom levels. It seems to add the element's insets (padding + border) to its width and height, although the CSS width already includes the insets.

Demonstration

http://output.jsbin.com/yipudeloya/

In the provided test case, we have two <div>s with 10px padding on all sides, 1px border on all sides and a CSS width/height set to 32px. Because box-sizing is set to border-box, the visual size of the elements is 32x32px with a remaining "inner" space of 10px. At 100% browser zoom level, this is correctly reported both by css('width') and .outerWidth().

If we now zoom to 110%, the numbers get slightly changed, probably due to some rounding errors (31.9886px instead of 32px). Because we should always get "CSS pixels" that are not affected by browser zoom, I would have expected those numbers to stay the same - but I can live with that. The real problem occurs when you reload the page while having the browser zoom at 110%! Now the width is completely incorrect (53.977236000000005px instead of 32px).

If the page uses those values for programmatic layout, the results will be wrong. You can see the effect in the demo. The CSS width and height of the first <div> are copied to the second, so they should always be the same size. However, if you start at 100%, the second <div> is much larger.

Analysis

Analysis has shown that this seems to be caused by the internal function computeStyleTests() in css/support.js that wrongly sets the internal property boxSizingReliableVal to false. Apparently, this happens because of a rounding error. In my case, the test div's measured width is 3.997px, which is not equal to the expected value of 4px. What's worse, this internal property is only calculated once! So when you start at 110% zoom, and then reset it to 100%, the problem still persists until you completely reload the page.

I could observe the problem at the following zoom levels: 110%, 90%, 80%, 67% and 33%.

Firefox (v56) and IE (v11) don't seem to be affected by this problem. Also, with jQuery 2.1.4, everything works fine on all browsers. All tests were performed on Windows 7 with default DPI and font size settings.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Oct 4, 2017

Ugh, confirmed on the git build. I think we need something like a unit-preserving round comparison. Fortunately, this is reproducible with html { zoom: 1.1 }, so we'll be able to verify a fix with unit testing.

@gibson042 gibson042 added CSS Dimensions and removed CSS labels Oct 4, 2017
@simonemazzoni
Copy link

@simonemazzoni simonemazzoni commented Oct 11, 2017

Same here, @gibson042 any news about it?
Up to now I fixed downgrading to version 3.1.1.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Oct 13, 2017

A PR would be welcome, but if nobody expresses interest then I'll probably put in a fix before the next release

@SaptakS
Copy link
Contributor

@SaptakS SaptakS commented Nov 6, 2017

@gibson042 I would like to work on this issue.

@timmywil
Copy link
Member

@timmywil timmywil commented Nov 6, 2017

@SaptakS Feel free!

hubot pushed a commit to eclipse/scout.rt that referenced this issue Nov 24, 2017
jquery/jquery#3808

Change-Id: I4b3b9dd60392a8724f4a660c9b7e4d6465762add
Reviewed-on: https://git.eclipse.org/r/112249
Tested-by: Hudson CI
Reviewed-by: Claudio Guglielmo <claudio.guglielmo@bsiag.com>
hubot pushed a commit to eclipse/scout.rt that referenced this issue Nov 24, 2017
jquery/jquery#3808

Change-Id: I4b3b9dd60392a8724f4a660c9b7e4d6465762add
Reviewed-on: https://git.eclipse.org/r/112249
Tested-by: Hudson CI
Reviewed-by: Claudio Guglielmo <claudio.guglielmo@bsiag.com>
(cherry picked from commit 778411b)
Reviewed-on: https://git.eclipse.org/r/112250
Tested-by: Claudio Guglielmo <claudio.guglielmo@bsiag.com>
@gibson042 gibson042 closed this in f00a075 Dec 6, 2017
@gibson042 gibson042 added this to the 3.3.0 milestone Dec 6, 2017
@cuzzins
Copy link

@cuzzins cuzzins commented Dec 6, 2017

Just to add to this, I reported this (or a very similar issue back in May) and was essentially told that browser zoom is not supported and I should essentially expect things to break around it between versions. Has the policy changed?

In the end I had to rip the code out that was using jQuery and rewrite it, and also have not been able to rely on jQuery height/width for other parts as it may break with no intention of fixing it.

See #3681 for what I originally described as the issue

@gibson042
Copy link
Member

@gibson042 gibson042 commented Dec 6, 2017

You didn't do anything wrong, but this particular issue was phrased in such a way and came with sufficient detail that it merited more attention. Even though both probably stemmed from similar underlying causes, this one highlighted a fundamental problem of our support tests failing with non-default zoom levels, which is more severe than any given response being wrong. And as @dmethvin noted, landing #3738 and #3656 changed things. We still can't guarantee behavior with non-default zoom, but as of f00a075 we have confidence that our browser behavior observations are correct (which is likely to resolve issues like #3681).

Our official policy remains "dimensions may be incorrect when the page is zoomed by the user", but we do try to make things work even outside of normal operating parameters if a fix can be achieved without incurring undue cost (factoring in all dimensions—file size, performance, code complexity, testability, maintenance burden, followup reports, etc.). I feel bad that you had to remove jQuery dimensions calls for your application, and hope that you will consider restoring them after 3.3 is released this month.

And please do report issues you encounter in the future. Our notices are caveats, not absolutes.

@bschwarzent
Copy link
Author

@bschwarzent bschwarzent commented Dec 12, 2017

@gibson042 Is there a way for me to verify the fix? I tried https://code.jquery.com/jquery-git.js, but that file does not seem to include the fix (yet?).

@gibson042
Copy link
Member

@gibson042 gibson042 commented Dec 13, 2017

Yes, that's correct. But it looks like the file is stuck: Date: 2017-11-28T12:34Z. @jquery/core, does someone need to kick Jenkins?

@mgol
Copy link
Member

@mgol mgol commented Dec 13, 2017

@gibson042 that's what our Jenkins looks like now most of the time:
screen shot 2017-12-13 at 09 32 52
Content jobs are broken & never end. I'm killing them all manually from time to time but I've been on vacation for the past few days; I've just done that and the jobs should go through.

@bschwarzent
Copy link
Author

@bschwarzent bschwarzent commented Dec 13, 2017

Thanks! I was able to successfully verify the fix with the updated jquery-git.js version. 👍

mvilliger added a commit to bsi-software/org.eclipse.scout.docs that referenced this issue May 2, 2018
hubot pushed a commit to eclipse/scout.rt that referenced this issue May 2, 2018
hfig pushed a commit to SydneyBoysHS/mrbs that referenced this issue May 24, 2018
… reports wrong width/height in Chrome on certain zoom levels).
@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.

7 participants