Modal: Work around IE scrollbars not taking up page width

[hnrch02]

Fixes #15353.

/cc @thcipriani

[cvrebert]

Which IE versions are affected?

[hnrch02]

10 and 11.

[hnrch02]

IE8 also had the same problem though there the scrollbar takes up window width, hence the workaround.

[thcipriani]

I can't seem to reproduce this in IE8.

Are you using getbootstrap.com? Or do you have a gist?

I actually think that you could drop document.body.scrollHeight > document.documentElement.clientHeight I can't think of a single situation where that check would correctly determine that there were scrollbars when the document.body.clientWidth < fullWindowWidth would disagree.

One caution I have, that I'm a bit confused about, in IE
document.documentElement.getBoundingClientRect().right - document.documentElement.getBoundingClientRect().left
DOES produce effectively the same number as window.innerWidth; however, in Chrome 39, it produces a number equvalent to document.documentElement.clientWdith—what's up with that?

[hnrch02]

I can't seem to reproduce this in IE8.

Are you using getbootstrap.com? Or do you have a gist?

I was using the visual JS test for the modal though I'm now also unable to reproduce. I think I was talking about another modal related problem.

I actually think that you could drop document.body.scrollHeight > document.documentElement.clientHeight I can't think of a single situation where that check would correctly determine that there were scrollbars when the document.body.clientWidth < fullWindowWidth would disagree.

Good point.

One caution I have, that I'm a bit confused about, in IE
document.documentElement.getBoundingClientRect().right - document.documentElement.getBoundingClientRect().left
DOES produce effectively the same number as window.innerWidth; however, in Chrome 39, it produces a number equvalent to document.documentElement.clientWdith—what's up with that?

I don't know but that's why the workaround method is only used when window.innerWidth is unavailable which is only the case in IE8.

[thcipriani]

My thought is if we can't confirm IE8 having the problem (or IE < 10) we can ditch the getBoundingClientRect() piece for now.

Then, once the regression is fixed via the merge, maybe try moving away from even looking to scrollHeight vs clientHeight.

Thoughts?

[hnrch02]

@thcipriani

Then, once the regression is fixed via the merge, maybe try moving away from even looking to scrollHeight vs clientHeight.

Isn't that what this PR would do? We'd be looking at the width instead of the height, or am I misunderstanding something here?

[thcipriani]

@hnrch02 yup, lgtm. We should merge a fix for this asap, been hanging out for quite a while

Thanks!

[cvrebert]

Apparently this also fixed #15718.