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

3.1.x -> 3.2.x contains breaking change (`outerWidth` behavior) #3611

Closed
jacobq opened this issue Apr 4, 2017 · 19 comments
Closed

3.1.x -> 3.2.x contains breaking change (`outerWidth` behavior) #3611

jacobq opened this issue Apr 4, 2017 · 19 comments
Assignees
Labels
Milestone

Comments

@jacobq
Copy link

@jacobq jacobq commented Apr 4, 2017

Please pardon this lengthy issue report. If you think it should be split into multiple separate issues please let me know and I will attempt to do this, but I'm not sure how these observations relate.

This may be related to #3589 or the fixes for #3193 mentioned in the 3.2.0 release notes, but I'm not sure. (I don't think it's related to the 3.0 breaking change since it worked in previous 3.x versions.)

Problem 1: Firefox: .width() & .outerWidth() return % values instead of px values for inline elements

Tested on v45.8.0 (ESR) on Linux as well as 51.0.1 (32-bit) on Windows 10 (64-bit)

Duplicate of #3571 Problem 2: Chrome: .width() returns 0 for inline elements

Problem 3: .width() / .outerWidth() gives different/wrong result on Chrome + Linux

This appears to be related to how padding is used in the calculation as well as to setting:

html {
  box-sizing: border-box;
}

*, *:before, *:after {
    box-sizing: inherit;
}

jquery-working-311
jquery-broken-latest
jquery-broken-firefox-latest
jquery-windows-working-latest

Background / Other Notes

This all started with a problem I had using Materialize. with the latest jQuery (3.2.1). The "tab indicator" suddenly started appearing too wide. You can see this here.
After digging into this a bit I found that:

  • It appears to affect Chrome on Linux (x86_64), not Firefox (ESR / 48.5.0) on Linux or common Windows browsers.
  • It works fine with jQuery 3.1.x but not 3.2.x (assuming semver compliance this should not happen)
  • Using .get(0).getBoundingClientRect().width instead of .outerWidth() avoided the problem
@Krinkle
Copy link
Member

@Krinkle Krinkle commented Apr 5, 2017

Problem 2 was indeed a regression (width/height broken for inline elements). Reported as issue #3571, since then resolved in master and released in jQuery 3.2.1 (see http://blog.jquery.com/2017/03/20/jquery-3-2-1-now-available/).

Although with notable exception that it is still broken on Android 4 - see issue #3602.

@jacobq
Copy link
Author

@jacobq jacobq commented Apr 5, 2017

@Krinkle Thanks for pointing that out. I updated my issue/comment to reflect this. I also spent some time this morning trying to figure out the root cause of the tabs issue (see notes at bottom of issue text) with the latest jQuery. I narrowed it down quite a bit and added links to jsbins as well as screenshots showing the discrepancies I observed in hopes that someone from the jQuery team will be able to identify what change caused this.

@timmywil
Copy link
Member

@timmywil timmywil commented Apr 10, 2017

@jacobq Thank you for the thorough analysis!

@timmywil timmywil added this to the 3.3.0 milestone Apr 10, 2017
@timmywil timmywil added the Blocker label Apr 10, 2017
@TimePerformance
Copy link

@TimePerformance TimePerformance commented Apr 24, 2017

Problem 4: outerHeight() returns wrong value for a <td> in IE 11 (Windows 10) but works fine in Chrome

@faller
Copy link

@faller faller commented May 26, 2017

I have the same problem like Problem 3

@smasala
Copy link

@smasala smasala commented Jul 25, 2017

@jacobq thanks for this (Problem 3). I can confirm it's the same problem under windows with FF 54.0.1 (64-bit) & Chrome 59.0.3071.115 (64-bit) and only for jQuery versions >=3.2.0

@smasala
Copy link

@smasala smasala commented Jul 25, 2017

For an even weirder version of this problem in chrome.... it seems dependant on the url structure.

Datatables plugin:

  1. clone https://github.com/smasala/ColResize
  2. npm install (make sure jQuery 3.2.0 or 3.2.1 is noted in package.json)
  3. open in chrome -> file:///C:/Temp/ColResize/examples/index.html
  4. make a copy of index.html -> index1.html
  5. open in chrome ->file:///C:/Temp/ColResize/examples/index1.html - here it works. (but index.html won't)

Make this even weirder:

  1. move both index.html and index1.html into a sub folder of examples.
  2. now both index.html and index1.html will render correctly
@jacobq
Copy link
Author

@jacobq jacobq commented Jul 25, 2017

@smasala Just for fun I tried the exercise you shared but saw no differences between index.html and index1.html. Perhaps this is because of platform differences (I am running Linux 4.9 on x86_64, node v6.11.1, and Chrome 59.0.3071.115 (Official Build) (64-bit)) or maybe it was because I wasn't sure what to look for.

Nevertheless, I would strongly encourage you to simplify the portion of code that you're analyzing. When there are lots of parts at work it becomes very easy to become mislead (e.g. accidental caching, race conditions, dependence on inputs / user behavior, etc. can make one believe the problem is caused by something other than the root cause), and there are a number of nuances one has to deal with regarding how a browser will behave differently for file:// URLs. Based on your description I suspect that the problem you're seeing is different from the ones described by the issue (and that it may not even be related to jQuery).

If using an older version of jQuery solves the problem in your application I would recommend doing that until a 3.3.x release is ready since it's most likely the best use of your time.

@smasala
Copy link

@smasala smasala commented Jul 25, 2017

@jacobq You would have seen the error if it hadn't have worked :)

I'm not looking for a solution with my plugin, just thought I'd post my random findings :) However, I found this issue because I narrowed the problem down to one line... jquery's outerWidth and only when box-sizing is set, after updating jQuery to the latest version (>=3.2.0) but was unable to replicate the issue with jsfiddle or similar. So it's good to have your links and examples above - I'm not going crazy after all :)

I've just tried #3611 (comment) on another machine and under Windows 10 (instead of 7) + Chrome it works fine too. Very odd. Perhaps there is something wrong with my SSD on the other machine. Will have to try it on other Win 7 setup just out of curiosity and a sanity check :) 👍

@smasala
Copy link

@smasala smasala commented Jul 26, 2017

@jacobq FYI :) works on other Win 7 / Chrome systems just fine. However, can be reproduced on my machine every time without fail. 3.1.1 works, 3.2.x produces the exact problem as you described above (problem 3). Also the case in Chrome 60.0.3112.78. I give up :)

@gibson042 gibson042 self-assigned this Jul 31, 2017
@gibson042
Copy link
Member

@gibson042 gibson042 commented Jul 31, 2017

As of a495784, everything is fixed except Problem 1.

@jacobq
Copy link
Author

@jacobq jacobq commented Aug 8, 2017

@gibson042 Thanks; this looks a lot better, though there is still a discrepancy. Any idea why my jsbin example (#3) gets all zeros with 3.1.1 but not with the latest (jQuery v3.2.2-pre 3cf1d14)? (In Chrome on Windows I see differences on the order of 0.48749542236328125)

@gibson042
Copy link
Member

@gibson042 gibson042 commented Aug 8, 2017

Yes, it's because 3.1.1 was using getBoundingClientRect but latest is actually calculating CSS boxes. Even though the latter can only approximate, the former is wrong because it is affected by transforms (scale, rotate, etc.).

@jacobq
Copy link
Author

@jacobq jacobq commented Aug 8, 2017

@gibson042
Copy link
Member

@gibson042 gibson042 commented Aug 8, 2017

I'd recommend that you instead plan to upgrade and use getBoundingClientRect directly if that's what your use case needs.

@gibson042
Copy link
Member

@gibson042 gibson042 commented Aug 8, 2017

I suppose it would also be possible for us to use the fractional component from gBCR, but that would be a separate issue:

val = elem[ "offset" + dimension[ 0 ].toUpperCase() + dimension.slice( 1 ) ];
if ( elem.getClientRects().length ) {
	val += ( elem.getBoundingClientRect()[ dimension[ 0 ] ] + 0.5 ) % 1 - 0.5;
}
@mgol
Copy link
Member

@mgol mgol commented Aug 8, 2017

@gibson042 That would be vulnerable to transforms as well, wouldn't it?

@gibson042
Copy link
Member

@gibson042 gibson042 commented Aug 8, 2017

Yes, but with a maximum error of half a pixel (i.e., no change from current) and recovering perfect accuracy in the absence of transforms.

@jacobq
Copy link
Author

@jacobq jacobq commented Jan 29, 2018

This looks like it is fixed in v3.3.x (i.e. in the jsbin we now have ow - gbcr < 1 again). Can anyone else confirm?

Update: Nevermind: I see it here. Thanks, core team + contributors!

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.

8 participants