[OS X] [RTL Scrollbars] List boxes should obey RTL scrollbars
Created attachment 275142 [details] Patch
Comment on attachment 275142 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=275142&action=review > Source/WebCore/rendering/RenderListBox.cpp:476 > + if (verticalScrollbarIsOnLeft() && (offset.width() < borderLeft() + paddingLeft() + scrollbarWidth || offset.width() > width() - borderRight() - paddingRight())) > + return -1; > + if (!verticalScrollbarIsOnLeft() && (offset.width() < borderLeft() + paddingLeft() || offset.width() > width() - borderRight() - paddingRight() - scrollbarWidth)) > return -1; Inelegant to have this in two separate if statements, but I have no cleaner suggestion. > Source/WebCore/rendering/RenderListBox.cpp:721 > - if (itemBoundingBoxRect(adjustedLocation, i).contains(locationInContainer.point())) { > - if (Element* node = listItems[i]) { > - result.setInnerNode(node); > - if (!result.innerNonSharedNode()) > - result.setInnerNonSharedNode(node); > - result.setLocalPoint(locationInContainer.point() - toLayoutSize(adjustedLocation)); > - break; > - } > + if (!itemBoundingBoxRect(adjustedLocation, i).contains(locationInContainer.point())) > + continue; > + if (Element* node = listItems[i]) { > + result.setInnerNode(node); > + if (!result.innerNonSharedNode()) > + result.setInnerNonSharedNode(node); > + result.setLocalPoint(locationInContainer.point() - toLayoutSize(adjustedLocation)); > + break; > } I like the new code slightly better, but why make this unrelated change in this patch. > Source/WebCore/rendering/RenderListBox.cpp:905 > bool RenderListBox::scrolledToTop() const > { > - Scrollbar* vbar = verticalScrollbar(); > - if (!vbar) > - return true; > - > + if (Scrollbar* vbar = verticalScrollbar()) > return vbar->value() <= 0; > + > + return true; > } > > bool RenderListBox::scrolledToBottom() const > { > - Scrollbar* vbar = verticalScrollbar(); > - if (!vbar) > - return true; > + if (Scrollbar* vbar = verticalScrollbar()) > + return vbar->value() >= vbar->maximum(); > > - return vbar->value() >= vbar->maximum(); > + return true; > } Old code with early return looks better to me.
Committed r198843: <http://trac.webkit.org/changeset/198843>
<rdar://problem/25137021>