<rdar://problem/8975367>
Created attachment 90683 [details] Patch
Comment on attachment 90683 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90683&action=review > Source/WebCore/platform/mac/ScrollbarThemeMac.mm:214 > + if (!scrollbar->parent()->isFrameView()) > + return ScrollbarOverlayStyleDefault; > + > + FrameView* frameView = static_cast<FrameView*>(scrollbar->parent()); > + Document* document = frameView->frame()->document(); > + Element* rootElementToUse = document->body(); > + if (!rootElementToUse) > + rootElementToUse = document->documentElement(); > + if (!rootElementToUse) > + return ScrollbarOverlayStyleDefault; > + > + RenderObject* renderer = rootElementToUse->renderer(); > + if (!renderer) > + return ScrollbarOverlayStyleDefault; > + Color color = renderer->style()->visitedDependentColor(CSSPropertyBackgroundColor); > + if (!color.isValid()) > + return ScrollbarOverlayStyleDefault; > + > + // reduce the background color from RGB to a lightness value > + // and determine which scrollbar style to use based on a magic > + // lightness heuristic > + double h, s, l; > + color.getHSL(h, s, l); > + if (l < .5) > + return ScrollbarOverlayStyleLight; This is a layering violation, since classes in WebCore/platform are not supposed to know about rendering, the DOM, or the frame tree. Instead, I think you should add a virtual method to ScrollableArea which FrameView can implement to return the background color. This will also make this possible to implement for overflow areas in the future.
Created attachment 90756 [details] Patch
Attachment 90756 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1 Source/WebCore/page/FrameView.cpp:332: l is incorrectly named. Don't use the single letter 'l' as an identifier name. [readability/naming] [4] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.
Created attachment 90758 [details] Patch
Comment on attachment 90758 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=90758&action=review > Source/WebCore/platform/ScrollableArea.cpp:201 > +ScrollbarOverlayStyle ScrollableArea::recommendedScrollbarOverlayStyle() Since this is a one-line function, it would be nice to have it implemented in the header. > Source/WebCore/platform/mac/ScrollbarThemeMac.mm:39 > + Unnecessary whitespace. You should remove this. > Source/WebCore/platform/mac/ScrollbarThemeMac.mm:-449 > - Arguably this whitespace was valuable. But I'll leave that up to you. :-) > Source/WebCore/platform/mac/WebCoreSystemInterface.h:230 > + Unnecessary tab-whitespace. Should remove.
Comment on attachment 90758 [details] Patch Oops, meant to r+ and cq- so that Jon can upload a new version with improved whitespace.
Created attachment 90784 [details] Patch
The commit-queue encountered the following flaky tests while processing attachment 90784 [details]: http/tests/xmlhttprequest/re-login.html bug 51987 (author: ap@webkit.org) The commit-queue is continuing to process your patch.
Comment on attachment 90784 [details] Patch Clearing flags on attachment: 90784 Committed r84740: <http://trac.webkit.org/changeset/84740>
All reviewed patches have been landed. Closing bug.
The commit-queue encountered the following flaky tests while processing attachment 90784 [details]: http/tests/xmlhttprequest/basic-auth.html bug 51613 (author: ap@webkit.org) http/tests/appcache/update-cache.html bug 52483 (author: ap@webkit.org) The commit-queue is continuing to process your patch.
I rolled this out via 59290, since it breaks internal builds and does not take into account feedback that I gave via email.
Created attachment 90897 [details] Patch
Comment on attachment 90897 [details] Patch r=me (previously reviewed by Simon and revised per his suggestions)
Comment on attachment 90897 [details] Patch Clearing flags on attachment: 90897 Committed r84769: <http://trac.webkit.org/changeset/84769>