Opened 10 years ago

Closed 10 years ago

#8763 closed bug (fixed)

Position: getScrollInfo() swapped width and height

Reported by: CR Owned by: Scott González
Priority: minor Milestone: 1.10.2
Component: ui.position Version: 1.9.1
Keywords: Cc:
Blocked by: Blocking:

Description

I wanted to use the scrollbar detection/calculation of jquery-ui-position and wondered why it switched the results.

In jquery.ui.position.js line 66 says:

return {
  width: hasOverflowX ? $.position.scrollbarWidth() : 0,
  height: hasOverflowY ? $.position.scrollbarWidth() : 0
};

If I have an overflowing content in horizontal (x) direction, I get a scrollbar on the bottom of the element, that means the height value has been modified. So in my opinion it should return the scrollbarwidth as the height attribute and not as the width.

Change History (12)

comment:1 Changed 10 years ago by Scott González

Owner: set to CR
Status: newpending

It looks to me like the code does exactly what you're saying. Also, it sounds like you're not even reporting a bug against position, but asking a question about how to use some of position's code outside of position. Can you please provide a reduced test case showing the problem you're having?

comment:2 Changed 10 years ago by CR

Status: pendingnew

Think of a simple page which has scrollbars on the right. That means, the content's height is too large to fit in. Code would correctly detect hasOverflowY=true and return somthing like {width:0,height:20}.

Now for example I would want to calculate the coordinates of an element to position it. And for that I would need the width of that (possible) scrollbar on the right. So I would expect to find this value in "width" attribute of the result instead of the "height".

I don't know If that affects other calculations in position or the "mind-switching" of the values is going through the rest of the code. For me this just doesn't seem to be right, maybe my way of thinking just differs from yours...

comment:3 Changed 10 years ago by Scott González

Resolution: notabug
Status: newclosed

It sounds like you don't have a bug to report. If you have a test case that is failing, please let us know.

comment:4 Changed 10 years ago by mikesherov

Resolution: notabug
Status: closedreopened

comment:5 Changed 10 years ago by mikesherov

I'd like to take a shot at proving the bug here. jsfiddle coming at some point some.

comment:6 Changed 10 years ago by mikesherov

Owner: changed from CR to mikesherov
Status: reopenedassigned

comment:7 Changed 10 years ago by Scott González

Sorry, reading this (and the code) again, it does sound like the logic might be reversed.

comment:8 Changed 10 years ago by Jörn Zaefferer

Switching around hasOverflowX and hasOverflowY doesn't seem to make a difference in practice, at least for the position utility. Does someone have a case where it actually produces the wrong result?

comment:9 Changed 10 years ago by mikesherov

That's what I'm looking into

comment:10 Changed 10 years ago by tj.vantoll

Milestone: 1.10.0none

comment:11 Changed 10 years ago by Scott González

Milestone: none1.10.2
Owner: changed from mikesherov to scott.gonzalez
Summary: jquery.ui.position.js getScrollInfo() swapped width and heightPosition: getScrollInfo() swapped width and height

comment:12 Changed 10 years ago by Scott González

Resolution: fixed
Status: assignedclosed

Position: Fix orientation check for scrollbar widths. Fixes #8763 - Position: getScrollInfo() swapped width and height.

Changeset: e9c04bfa430eb6b18e7fe1be2f8d162e01181a94

Note: See TracTickets for help on using tickets.