Skip to content

Commit

Permalink
Fixes #1450: a scroll view with non hiding legacy scrollbars would cr…
Browse files Browse the repository at this point in the history
…ash if the document view had a size of 0 pixels in either dimension.
  • Loading branch information
aljungberg committed Feb 8, 2012
1 parent c9d4089 commit fe70a55
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 2 deletions.
7 changes: 5 additions & 2 deletions AppKit/CPScrollView.j
Original file line number Diff line number Diff line change
Expand Up @@ -1138,8 +1138,9 @@ var CPScrollerStyleGlobal = CPScrollerStyleOverlay,
if (_scrollerStyle === CPScrollerStyleOverlay && hasHorizontalScroll)
verticalScrollerHeight -= horizontalScrollerHeight;

var documentHeight = _CGRectGetHeight(documentFrame);
[_verticalScroller setFloatValue:(difference.height <= 0.0) ? 0.0 : scrollPoint.y / difference.height];
[_verticalScroller setKnobProportion:_CGRectGetHeight(contentFrame) / _CGRectGetHeight(documentFrame)];
[_verticalScroller setKnobProportion:documentHeight > 0 ? _CGRectGetHeight(contentFrame) / documentHeight : 1.0];
[_verticalScroller setFrame:_CGRectMake(_CGRectGetMaxX(contentFrame) - overlay, verticalScrollerY, verticalScrollerWidth, verticalScrollerHeight)];
}
else if (wasShowingVerticalScroller)
Expand All @@ -1155,8 +1156,10 @@ var CPScrollerStyleGlobal = CPScrollerStyleOverlay,
if (_scrollerStyle === CPScrollerStyleOverlay && hasVerticalScroll)
horizontalScrollerWidth -= verticalScrollerWidth;

var documentWidth = _CGRectGetWidth(documentFrame);

[_horizontalScroller setFloatValue:(difference.width <= 0.0) ? 0.0 : scrollPoint.x / difference.width];
[_horizontalScroller setKnobProportion:_CGRectGetWidth(contentFrame) / _CGRectGetWidth(documentFrame)];
[_horizontalScroller setKnobProportion:documentWidth > 0 ? _CGRectGetWidth(contentFrame) / documentWidth : 1.0];
[_horizontalScroller setFrame:_CGRectMake(_CGRectGetMinX(contentFrame), _CGRectGetMaxY(contentFrame) - overlay, horizontalScrollerWidth, horizontalScrollerHeight)];
}
else if (wasShowingHorizontalScroller)
Expand Down
16 changes: 16 additions & 0 deletions Tests/AppKit/CPScrollViewTest.j
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,21 @@
{
}

/*!
Test that scroll views don't generate bad layouts when very small.
*/
- (void)testZeroSize
{
var scrollView = [[CPScrollView alloc] initWithFrame:CGRectMakeZero()],
documentView = [[CPView alloc] initWithFrame:CGRectMake(0, 0, 0, 0)];
[scrollView setAutohidesScrollers:NO];
[scrollView setDocumentView:documentView];
[scrollView setScrollerStyle:CPScrollerStyleLegacy];
// If the layout operation leads to setKnobProportion:0/0 this will crash.
[[scrollView horizontalScroller] layoutSubviews];
[[scrollView verticalScroller] layoutSubviews];
}

- (void)testBothScrollersVisible
{
var scrollView = [[CPScrollView alloc] initWithFrame:CGRectMake(0.0, 0.0, 100.0, 100.0)];
Expand Down Expand Up @@ -200,3 +215,4 @@
}

@end

5 comments on commit fe70a55

@Me1000
Copy link

@Me1000 Me1000 commented on fe70a55 Feb 8, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

He do you feel about a quick release for this fix?

@Me1000
Copy link

@Me1000 Me1000 commented on fe70a55 Feb 8, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps I don't understand what this fixes... Does it fix everyone's scrollviews issues with the latest release?

@aljungberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes a very specific issue with e.g. a document size with CGRectMakeZero frame. What issue were you thinking about?

@Me1000
Copy link

@Me1000 Me1000 commented on fe70a55 Feb 8, 2012

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All those people saying scrollviews don't when then upgraded to 0.9.5

@aljungberg
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I didn't follow that discussion. All I can say is that before this fix scrollbars worked OK for me in IE, and after the fix they're now OKer.

Please sign in to comment.