Bug 64220 - Scrollbar color heuristic needs to be hooked up in WebKit1
: Scrollbar color heuristic needs to be hooked up in WebKit1
Status: RESOLVED FIXED
: WebKit
WebCore Misc.
: 528+ (Nightly build)
: Macintosh Unspecified
: P2 Normal
Assigned To:
:
: InRadar
:
:
  Show dependency treegraph
 
Reported: 2011-07-08 17:24 PST by
Modified: 2011-07-21 12:40 PST (History)


Attachments
Patch (1.82 KB, patch)
2011-07-08 17:46 PST, Tim Horton
no flags Review Patch | Details | Formatted Diff | Diff
totally new patch, utilizing Darin's suggestions (22.88 KB, patch)
2011-07-18 15:03 PST, Tim Horton
no flags Review Patch | Details | Formatted Diff | Diff
same as last + radar URL (22.98 KB, patch)
2011-07-18 15:05 PST, Tim Horton
gyuyoung.kim: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
this time, without breaking all of the other platforms! (23.21 KB, patch)
2011-07-18 15:40 PST, Tim Horton
no flags Review Patch | Details | Formatted Diff | Diff
work around <rdar://problem/9797253> (24.77 KB, patch)
2011-07-18 17:34 PST, Tim Horton
darin: review+
webkit.review.bot: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
remove updateStyleIfNeeded() (24.66 KB, patch)
2011-07-19 10:40 PST, Tim Horton
no flags Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-07-08 17:24:43 PST
We don't currently set the color of the scrollbar based on the content for WebKit1 views.

<rdar://problem/9589140>

Patch forthcoming.
------- Comment #1 From 2011-07-08 17:46:48 PST -------
Created an attachment (id=100192) [details]
Patch

Not commit-worthy yet, I don't think that putting this in repaint is the right place, though it's not clear there's a better place.
------- Comment #2 From 2011-07-11 17:02:55 PST -------
(From update of attachment 100192 [details])
Looking back at the implementation of this that is already checked in:

I was surprised to see that in ScrollbarThemeMac::paint, the knob style is recomputed each time we draw a scrollbar. That is not a good design. Even if setting the style on the painter each time was efficient enough, it’s not conceptually OK to simply draw the scrollbar in a different style without making sure we repaint existing visible scrollbars when the style changes. Generally speaking such things are set up as part of style computation and layout.

It’s also not good that we added a getDocumentBackgroundColor function to Frame. This is not the right class for a function like that. For one thing, we should not be adding new functions to the Frame class at this time since it is just the center of a set of related classes. It is probably reasonable to put this function in the FrameView class.

The name of the getDocumentBackgroundColor function, starting with get, is correct style inside WebKit2, but not correct WebCore. The get prefix should have been removed.

The getDocumentBackgroundColor function gets at renderers without triggering style recalculation first. This is not correct unless the caller somehow guarantees rendering is up to date. Normally a function like this needs to make sure style is calculated before looking at renderers or must only be called when style is guaranteed to be up to date.

After talking to Maciej we agree that the right design for this is to store the scrollbar overlay style in the ScrollableArea. If the scrollbar overlay style changes, the scrollable area then schedules a scrollbar repaint. In the case of a scrollable area that is implemented with ScrollView, it would call a function from the ScrollView class.

We’d also need code to schedule a scrollbar overlay style recalculation that works either immediately or based on a timer. That would be called by any code that could change the background color. This would need to be done any time the color of the document element or the body element changes, presumably in RenderBox::styleDidChange, or any time the view's base background color changes, which would be in FrameView::setBaseBackgroundColor.
------- Comment #3 From 2011-07-15 11:31:22 PST -------
(In reply to comment #2)
Ok, I'm a lot more familiar with the scrolling code after the last two days, so I'll take a crack at this and see what happens. Thanks, Darin!
------- Comment #4 From 2011-07-18 15:03:48 PST -------
Created an attachment (id=101212) [details]
totally new patch, utilizing Darin's suggestions
------- Comment #5 From 2011-07-18 15:05:59 PST -------
Created an attachment (id=101213) [details]
same as last + radar URL
------- Comment #6 From 2011-07-18 15:22:06 PST -------
(From update of attachment 101213 [details])
Attachment 101213 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9108002
------- Comment #7 From 2011-07-18 15:32:52 PST -------
(From update of attachment 101213 [details])
Attachment 101213 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9117373
------- Comment #8 From 2011-07-18 15:37:00 PST -------
(From update of attachment 101213 [details])
Attachment 101213 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/9100022
------- Comment #9 From 2011-07-18 15:40:03 PST -------
Created an attachment (id=101224) [details]
this time, without breaking all of the other platforms!
------- Comment #10 From 2011-07-18 16:45:10 PST -------
Interesting. Works great for WK1, but for WK2, somewhere between setting the correct overlay style (I've added an updateScrollbarOverlayStyle call to registerScrollbar and setNewPainterForScrollbar) and going to paint, the NSScrollerImp is losing its style (getting reset to default). Will continue investigating (if all else fails, we could just set the cached style during every paint(); we still get 99% of the benefit).
------- Comment #11 From 2011-07-18 16:49:28 PST -------
(From update of attachment 101224 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=101224&action=review

> Source/WebCore/page/FrameView.cpp:333
> +    Color bgColor = documentBackgroundColor();

I’d suggest not abbreviated background to “bg”.

> Source/WebCore/page/FrameView.cpp:2390
> +    // start as invalid colors

I’d suggest changing this comment to sentence style:

    // Start with invalid colors.

> Source/WebCore/platform/ScrollView.cpp:790
> +    if (horizontalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(horizontalScrollbar());
> +    if (verticalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());

Why is this code here instead of in the ScrollableArea base class?

> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:239
> +    switch (style) {
> +        case ScrollbarOverlayStyleDark:
> +            return wkScrollerKnobStyleDark;
> +        case ScrollbarOverlayStyleLight:
> +            return wkScrollerKnobStyleLight;
> +        default:
> +            return wkScrollerKnobStyleDefault;
> +    }

The old indenting style is the correct style for WebKit, believe it or not.
------- Comment #12 From 2011-07-18 16:54:49 PST -------
(From update of attachment 101224 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=101224&action=review

>> Source/WebCore/page/FrameView.cpp:333
>> +    Color bgColor = documentBackgroundColor();
> 
> I’d suggest not abbreviated background to “bg”.

Ok.

>> Source/WebCore/page/FrameView.cpp:2390
>> +    // start as invalid colors
> 
> I’d suggest changing this comment to sentence style:
> 
>     // Start with invalid colors.

Ok, sure!

>> Source/WebCore/platform/ScrollView.cpp:790
>> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());
> 
> Why is this code here instead of in the ScrollableArea base class?

Fixed.

>> Source/WebCore/platform/mac/ScrollbarThemeMac.mm:239
>> +    }
> 
> The old indenting style is the correct style for WebKit, believe it or not.

That's odd; does Xcode reindent code when you copy/paste it around?
------- Comment #13 From 2011-07-18 17:34:33 PST -------
Created an attachment (id=101242) [details]
work around <rdar://problem/9797253>
------- Comment #14 From 2011-07-18 18:12:45 PST -------
(From update of attachment 101242 [details])
Attachment 101242 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9112503

New failing tests:
fast/css/stylesheet-candidate-nodes-crash.xhtml
------- Comment #15 From 2011-07-18 18:25:35 PST -------
(From update of attachment 101242 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=101242&action=review

I think the EWS failure was a false positive.

> Source/WebCore/platform/ScrollableArea.cpp:227
> +    if (horizontalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(horizontalScrollbar());
> +    if (verticalScrollbar())
> +        ScrollbarTheme::nativeTheme()->updateScrollbarOverlayStyle(verticalScrollbar());
> +
> +    if (horizontalScrollbar())
> +        horizontalScrollbar()->invalidate();
> +
> +    if (verticalScrollbar())
> +        verticalScrollbar()->invalidate();

Could we merge these if statements for clarity?

> Source/WebCore/platform/mac/ScrollViewMac.mm:215
> +    switch (style) {
> +        case ScrollbarOverlayStyleDark:
> +            return NSScrollerKnobStyleDark;
> +        case ScrollbarOverlayStyleLight:
> +            return NSScrollerKnobStyleLight;
> +        default:
> +            return NSScrollerKnobStyleDefault;
> +    }

Same switch statement indentation problem.
------- Comment #16 From 2011-07-18 19:27:12 PST -------
(From update of attachment 101242 [details])
Rejecting attachment 101242 [details] from commit-queue.

Failed to run "['./Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '--bot-id=ec2-cq-01', '--port..." exit_code: 2

Last 500 characters of output:

  perf/object-keys.html = TEXT PASS


Regressions: Unexpected image mismatch : (5)
  fast/text/atsui-multiple-renderers.html = IMAGE
  fast/text/international/danda-space.html = IMAGE
  fast/text/international/thai-baht-space.html = IMAGE
  fast/text/international/thai-line-breaks.html = IMAGE
  platform/chromium-linux/fast/text/international/complex-joining-using-gpos.html = IMAGE

Regressions: Unexpected DumpRenderTree crashes : (1)
  fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH



Full output: http://queues.webkit.org/results/9136008
------- Comment #17 From 2011-07-19 10:23:42 PST -------
(In reply to comment #16)

While the rest of the failures seem to be happening to everyone, this one:

fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH

seems to be legitimate (can reproduce with patch, cannot without). But the backtrace certainly doesn't make it obvious how this patch is causing this failure. I'll take a look.
------- Comment #18 From 2011-07-19 10:32:28 PST -------
(In reply to comment #17)
> (In reply to comment #16)
> 
> While the rest of the failures seem to be happening to everyone, this one:
> 
> fast/css/stylesheet-candidate-nodes-crash.xhtml = CRASH
> 
> seems to be legitimate (can reproduce with patch, cannot without). But the backtrace certainly doesn't make it obvious how this patch is causing this failure. I'll take a look.

It was the updateStyleIfNeeded() added to FrameView::documentBackgroundColor.
------- Comment #19 From 2011-07-19 10:33:48 PST -------
(In reply to comment #18)
> It was the updateStyleIfNeeded() added to FrameView::documentBackgroundColor.

Very interesting. Maybe we need to make that change all alone in a separate patch and figure out what callers have difficulty.

I guess that could be done *after* this fix if the fix works without it. Or *before* if it really doesn't.
------- Comment #20 From 2011-07-19 10:40:34 PST -------
Created an attachment (id=101341) [details]
remove updateStyleIfNeeded()

> I guess that could be done *after* this fix if the fix works without it. Or *before* if it really doesn't.

Everything works fine without it as far as my manual tests go (I added it solely because of your comment, though it certainly makes sense), so my vote is for solve that mystery after this patch is landed.
------- Comment #21 From 2011-07-20 18:32:30 PST -------
(From update of attachment 101341 [details])
Clearing flags on attachment: 101341

Committed r91435: <http://trac.webkit.org/changeset/91435>
------- Comment #22 From 2011-07-20 18:32:36 PST -------
All reviewed patches have been landed.  Closing bug.
------- Comment #23 From 2011-07-20 18:49:54 PST -------
It seems like this broke Snow Leopard build:
http://build.webkit.org/builders/SnowLeopard%20Intel%20Release%20%28Build%29/builds/32163
------- Comment #24 From 2011-07-20 23:12:39 PST -------
This patch appears to have caused valgrind error:

19:02:10 memcheck_analyze.py [ERROR] FAIL! There were 1 errors: 
19:02:10 memcheck_analyze.py [ERROR] Command: src/sconsbuild/Release/test_shell_tests --gtest_filter=-BMPImageDecoderTest.FLAKY_ChunkedDecodingSlow:MediaLeakTest.VideoBear:MediaLeakTest.FAILS_VideoBear:WebFrameTest.GetFullHtmlOfPage:BMPImageDecoderTest.FLAKY_DecodingSlow:BMPImageDecoderTest.FAILS_ChunkedDecodingSlow:MediaLeakTest.FLAKY_VideoBear:MediaLeakTest.ManyVideoBear:BMPImageDecoderTest.DecodingSlow:MediaLeakTest.FAILS_ManyVideoBear:WebFrameTest.FAILS_GetFullHtmlOfPage:MediaLeakTest.FLAKY_ManyVideoBear:BMPImageDecoderTest.ChunkedDecodingSlow:BMPImageDecoderTest.FAILS_DecodingSlow:WebFrameTest.FLAKY_GetFullHtmlOfPage --gtest_print_time
UninitCondition
Conditional jump or move depends on uninitialised value(s)
  WebCore::FrameView::recalculateScrollbarOverlayStyle() (third_party/WebKit/Source/WebCore/page/FrameView.cpp:344)
  WebCore::RenderBox::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) (third_party/WebKit/Source/WebCore/rendering/RenderBox.cpp:341)
  WebCore::RenderBlock::styleDidChange(WebCore::StyleDifference, WebCore::RenderStyle const*) (third_party/WebKit/Source/WebCore/rendering/RenderBlock.cpp:233)
  WebCore::RenderObject::setStyle(WTF::PassRefPtr<WebCore::RenderStyle>) (third_party/WebKit/Source/WebCore/rendering/RenderObject.cpp:1631)
  WebCore::RenderObject::setAnimatableStyle(WTF::PassRefPtr<WebCore::RenderStyle>) (third_party/WebKit/Source/WebCore/rendering/RenderObject.cpp:1544)
  WebCore::NodeRendererFactory::createRendererAndStyle() (third_party/WebKit/Source/WebCore/dom/NodeRenderingContext.cpp:287)
  WebCore::NodeRendererFactory::createRendererIfNeeded() (third_party/WebKit/Source/WebCore/dom/NodeRenderingContext.cpp:316)
  WebCore::Node::createRendererIfNeeded() (third_party/WebKit/Source/WebCore/dom/Node.cpp:1454)
  WebCore::Element::attach() (third_party/WebKit/Source/WebCore/dom/Element.cpp:1015)
  WTF::PassRefPtr<WebCore::Element> WebCore::HTMLConstructionSite::attach<WebCore::Element>(WebCore::ContainerNode*, WTF::PassRefPtr<WebCore::Element>) (third_party/WebKit/Source/WebCore/html/parser/HTMLConstructionSite.cpp:111)
  WebCore::HTMLConstructionSite::insertHTMLHtmlStartTagBeforeHTML(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLConstructionSite.cpp:186)
  WebCore::HTMLTreeBuilder::defaultForBeforeHTML() (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2685)
  WebCore::HTMLTreeBuilder::processEndOfFile(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:2594)
  WebCore::HTMLTreeBuilder::processToken(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:493)
  WebCore::HTMLTreeBuilder::constructTreeFromAtomicToken(WebCore::AtomicHTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:462)
  WebCore::HTMLTreeBuilder::constructTreeFromToken(WebCore::HTMLToken&) (third_party/WebKit/Source/WebCore/html/parser/HTMLTreeBuilder.cpp:452)
  WebCore::HTMLDocumentParser::pumpTokenizer(WebCore::HTMLDocumentParser::SynchronousMode) (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:276)
  WebCore::HTMLDocumentParser::pumpTokenizerIfPossible(WebCore::HTMLDocumentParser::SynchronousMode) (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:175)
  WebCore::HTMLDocumentParser::prepareToStopParsing() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:140)
  WebCore::HTMLDocumentParser::attemptToEnd() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:399)
  WebCore::HTMLDocumentParser::finish() (third_party/WebKit/Source/WebCore/html/parser/HTMLDocumentParser.cpp:426)
  WebCore::DocumentWriter::endIfNotLoadingMainResource() (third_party/WebKit/Source/WebCore/loader/DocumentWriter.cpp:226)
  WebCore::DocumentWriter::end() (third_party/WebKit/Source/WebCore/loader/DocumentWriter.cpp:209)
  WebCore::FrameLoader::init() (third_party/WebKit/Source/WebCore/loader/FrameLoader.cpp:233)
  WebCore::Frame::init() (third_party/WebKit/Source/WebCore/page/Frame.h:286)
  WebKit::WebFrameImpl::initializeAsMainFrame(WebKit::WebViewImpl*) (third_party/WebKit/Source/WebKit/chromium/src/WebFrameImpl.cpp:1883)
  WebKit::WebViewImpl::initializeMainFrame(WebKit::WebFrameClient*) (third_party/WebKit/Source/WebKit/chromium/src/WebViewImpl.cpp:271)
  WebViewHost::Create(_GtkWidget*, TestWebViewDelegate*, WebKit::WebDevToolsAgentClient*, WebPreferences const&) (webkit/tools/test_shell/webview_host_gtk.cc:34)
  TestShell::Initialize(GURL const&) (webkit/tools/test_shell/test_shell_gtk.cc:349)
  TestShell::CreateNewWindow(GURL const&, TestShell**) (webkit/tools/test_shell/test_shell.cc:203)
Suppression (error hash=#1E5287B6D215189B#):
  For more info on using suppressions see http://dev.chromium.org/developers/how-tos/using-valgrind#TOC-Suppressing-Errors
{
   <insert_a_suppression_name_here>
   Memcheck:Cond
   fun:_ZN7WebCore9FrameView32recalculateScrollbarOverlayStyleEv
   fun:_ZN7WebCore9RenderBox14styleDidChangeENS_15StyleDifferenceEPKNS_11RenderStyleE
   fun:_ZN7WebCore11RenderBlock14styleDidChangeENS_15StyleDifferenceEPKNS_11RenderStyleE
   fun:_ZN7WebCore12RenderObject8setStyleEN3WTF10PassRefPtrINS_11RenderStyleEEE
   fun:_ZN7WebCore12RenderObject18setAnimatableStyleEN3WTF10PassRefPtrINS_11RenderStyleEEE
   fun:_ZN7WebCore19NodeRendererFactory22createRendererAndStyleEv
   fun:_ZN7WebCore19NodeRendererFactory22createRendererIfNeededEv
   fun:_ZN7WebCore4Node22createRendererIfNeededEv
   fun:_ZN7WebCore7Element6attachEv
   fun:_ZN7WebCore20HTMLConstructionSite6attachINS_7ElementEEEN3WTF10PassRefPtrIT_EEPNS_13ContainerNodeES6_
   fun:_ZN7WebCore20HTMLConstructionSite32insertHTMLHtmlStartTagBeforeHTMLERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder20defaultForBeforeHTMLEv
   fun:_ZN7WebCore15HTMLTreeBuilder16processEndOfFileERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder12processTokenERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder28constructTreeFromAtomicTokenERNS_15AtomicHTMLTokenE
   fun:_ZN7WebCore15HTMLTreeBuilder22constructTreeFromTokenERNS_9HTMLTokenE
   fun:_ZN7WebCore18HTMLDocumentParser13pumpTokenizerENS0_15SynchronousModeE
   fun:_ZN7WebCore18HTMLDocumentParser23pumpTokenizerIfPossibleENS0_15SynchronousModeE
   fun:_ZN7WebCore18HTMLDocumentParser20prepareToStopParsingEv
   fun:_ZN7WebCore18HTMLDocumentParser12attemptToEndEv
   fun:_ZN7WebCore18HTMLDocumentParser6finishEv
   fun:_ZN7WebCore14DocumentWriter27endIfNotLoadingMainResourceEv
}
------- Comment #25 From 2011-07-20 23:13:15 PST -------
It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
------- Comment #26 From 2011-07-20 23:16:53 PST -------
(In reply to comment #25)
> It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(

Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.

Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)
------- Comment #27 From 2011-07-20 23:27:20 PST -------
(In reply to comment #26)
> (In reply to comment #25)
> > It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
> 
> Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.

Indeed!  Fixed in http://trac.webkit.org/changeset/91447.

> Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)

There's a documentation on chromium.org: http://www.chromium.org/developers/how-tos/using-valgrind

But I've never used it myself so I'm not sure how much of that is applicable to Mac port.

If anything, Chromium Linux has Valgrind bot:
http://build.chromium.org/p/chromium.webkit/waterfall?builder=Linux%20Valgrind

We also have a shiny Snow Leopard Clang bot:
http://build.chromium.org/p/chromium.webkit/waterfall?builder=Mac%20Clang%20Builder%20%28dbg%29
------- Comment #28 From 2011-07-20 23:29:03 PST -------
(In reply to comment #27)
> (In reply to comment #26)
> > (In reply to comment #25)
> > > It appears that m_scrollbarOverlayStyle added to ScrollableArea is never initialized :(
> > 
> > Eek! It needs to be initialized to ScrollbarOverlayStyleDefault. I'll do that tomorrow when I get back to my machine.
> 
> Indeed!  Fixed in http://trac.webkit.org/changeset/91447.

Ah, you've been great today, cleaning up after all my mistakes. Thanks!

> > Is there a way to run valgrind tests on my machine? (though this one should have been caught simply by not making the mistake in the first place)
> 
> There's a documentation on chromium.org: http://www.chromium.org/developers/how-tos/using-valgrind
> 
> But I've never used it myself so I'm not sure how much of that is applicable to Mac port.
> 
> If anything, Chromium Linux has Valgrind bot:
> http://build.chromium.org/p/chromium.webkit/waterfall?builder=Linux%20Valgrind
> 
> We also have a shiny Snow Leopard Clang bot:
> http://build.chromium.org/p/chromium.webkit/waterfall?builder=Mac%20Clang%20Builder%20%28dbg%29

Excellent, I'll take a look!
------- Comment #29 From 2011-07-21 12:40:33 PST -------
(From update of attachment 101341 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=101341&action=review

> Source/WebCore/ChangeLog:14
> +        No new tests, since this code is only enabled on future versions of Mac OS X.

No longer future!