[webkit-reviews] review granted: [Bug 36125] -[WebFrame setAlwaysHideHorizontal/VerticalScroller:] prevents keyboard scrolling : [Attachment 50720] Patch that still allows keyboard scrolling in WebFrameView after setAlwaysHideHorizontal/VerticalScroller:YES has been called.

[bugzilla-daemon at webkit.org]

Adam Roben (aroben) <aroben at apple.com> has granted John Sullivan
<sullivan at apple.com>'s request for review:
Bug 36125: -[WebFrame setAlwaysHideHorizontal/VerticalScroller:] prevents
keyboard scrolling
https://bugs.webkit.org/show_bug.cgi?id=36125

Attachment 50720: Patch that still allows keyboard scrolling in WebFrameView
after setAlwaysHideHorizontal/VerticalScroller:YES has been called.
https://bugs.webkit.org/attachment.cgi?id=50720&action=review

------- Additional Comments from Adam Roben (aroben) <aroben at apple.com>
> +	   (-[WebFrameView _isScrollable]):
> +	   New method, calls -[WebDynamicScrollBarsView
horizontalScrollingAllowed] and
> +	   -[WebDynamicScrollBarsView verticalScrollingAllowed]
> +	   (-[WebFrameView _largestScrollableChild]):
> +	   New method, like _largestChildWithScrollBars but uses _isScrollable.

> +	   (-[WebFrameView _hasScrollBars]):
> +	   Added a comment that this can probably now be deleted. (I'll do so
separately.)
> +	   (-[WebFrameView _largestChildWithScrollBars]):
> +	   Ditto.

If you were to rename these methods in place, it would be a lot easier to see
what your changes are. Then you could move them into their sorted location
separately.

> @@ -169,11 +179,16 @@ - (void)updateScrollers
>	   newHasHorizontalScroller = (hScroll == ScrollbarAlwaysOn);
>      if (vScroll != ScrollbarAuto)
>	   newHasVerticalScroller = (vScroll == ScrollbarAlwaysOn);
> -	   
> -    newHasHorizontalScroller &= !hideHorizontalScroller;
> -    newHasVerticalScroller &= !hideVerticalScroller;
>      
>      if (!documentView || suppressLayout || suppressScrollers || (hScroll !=
ScrollbarAuto && vScroll != ScrollbarAuto)) {
> +	   horizontalScrollingAllowedButScrollerHidden =
newHasHorizontalScroller && alwaysHideHorizontalScroller;
> +	   if (horizontalScrollingAllowedButScrollerHidden)
> +	       newHasHorizontalScroller = NO;
> +	   
> +	   verticalScrollingAllowedButScrollerHidden = newHasVerticalScroller
&& alwaysHideVerticalScroller;
> +	   if (verticalScrollingAllowedButScrollerHidden)
> +	       newHasVerticalScroller = NO;
> +	   
>	   inUpdateScrollers = YES;
>	   if (hasHorizontalScroller != newHasHorizontalScroller)
>	       [self setHasHorizontalScroller:newHasHorizontalScroller];
> @@ -212,9 +227,14 @@ - (void)updateScrollers
>      if (!newHasVerticalScroller && hasVerticalScroller && hScroll !=
ScrollbarAlwaysOn)
>	   newHasHorizontalScroller = NO;
>  
> -    newHasHorizontalScroller &= !hideHorizontalScroller;
> -    newHasVerticalScroller &= !hideVerticalScroller;
> -
> +    horizontalScrollingAllowedButScrollerHidden = newHasHorizontalScroller
&& alwaysHideHorizontalScroller;
> +    if (horizontalScrollingAllowedButScrollerHidden)
> +	   newHasHorizontalScroller = NO;
> +    
> +    verticalScrollingAllowedButScrollerHidden = newHasVerticalScroller &&
alwaysHideVerticalScroller;
> +    if (verticalScrollingAllowedButScrollerHidden)
> +	   newHasVerticalScroller = NO;
> +	   

It's too bad this code is repeated.

r=me