Webkit setting for embedders that do not support fullscreen.

Webkit setting for embedders that do not support fullscreen.

The new setting is fullscreenEnabled, but in this change
we have also added tests for
disallowFullscreenForNonMediaElements.

The motivation for this change is to support Android
WebView apps that do not implement WebChromeClient.onShowCustomView.
onShowCustomView is required to support fullscreen. When not supported,
we need to stop going fullscreen at the beginning of
FullscreenElementStack::requestFullScreenForElement
to avoid side effects and changes to the layout.

This failure to enter fullscreen falls into the following
condition on the living standard:
"There is a previously-established user preference, 
security risk, or platform limitation."
http://fullscreen.spec.whatwg.org/

BUG=389496
Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=179470

[Ignacio Solla, 2014-07-28 17:44:15]

dglazkov@chromium.org: Please review changes in

[philipj_slow, 2014-07-29 11:22:53]

Thanks for following up on this!

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
File Source/core/dom/FullscreenElementStack.cpp (right):

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:151: if
(!document()->frameHost()->chrome().client().fullscreenEnabled(&element)) {
This needs to be in the do { } while(0) loop below to trigger the
fullscreenerror event.

I also think you should just add a setting in Settings.in, which will be
required for testing and should allow you to remove all code except here,
Settings and WebSettings(Impl).

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:269: 
stray newline

[philipj_slow, 2014-07-29 11:23:28]

+falken, who has been doing a lot of fullscreen reviewing lately.

[Ignacio Solla, 2014-07-29 12:43:31]

Please hold on the review until I upload a new version, which will also add
tests.

[Ignacio Solla, 2014-07-29 18:05:58]

PTAL. Thanks

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
File Source/core/dom/FullscreenElementStack.cpp (right):

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:151: if
(!document()->frameHost()->chrome().client().fullscreenEnabled(&element)) {
On 2014/07/29 11:22:53, philipj wrote:
> This needs to be in the do { } while(0) loop below to trigger the
> fullscreenerror event.
> 
> I also think you should just add a setting in Settings.in, which will be
> required for testing and should allow you to remove all code except here,
> Settings and WebSettings(Impl).

I have moved this inside the while loop as you recommended. Also note that this
change probably falls into the following category in the spec:
"There is a previously-established user preference, security risk, or platform
limitation."

I have also added settings for both fullscreenEnabled and
disallowFullscreenForNonMediaElements, and also tests. I have also removed all
the unnecessary code after this change.

https://codereview.chromium.org/428633004/diff/1/Source/core/dom/FullscreenEl...
Source/core/dom/FullscreenElementStack.cpp:269: 
On 2014/07/29 11:22:53, philipj wrote:
> stray newline

Done.

[philipj_slow, 2014-07-30 11:53:13]

The description links to
https://code.google.com/p/chromium/issues/detail?id=387898 but that doesn't seem
right, did you paste the wrong bug number?

[philipj_slow, 2014-07-30 11:54:26]

Since you said "W3C spec" it sounds like you've found a stable spec. This is the
one that's maintained:
http://fullscreen.spec.whatwg.org

It says the same thing, though, so it doesn't matter.

[Ignacio Solla, 2014-07-30 12:09:00]

On 2014/07/30 11:53:13, philipj wrote:
> The description links to
> https://code.google.com/p/chromium/issues/detail?id=387898 but that doesn't
seem
> right, did you paste the wrong bug number?

Yes, fixed now.

[Ignacio Solla, 2014-07-30 12:10:17]

On 2014/07/30 11:54:26, philipj wrote:
> Since you said "W3C spec" it sounds like you've found a stable spec. This is
the
> one that's maintained:
> http://fullscreen.spec.whatwg.org
> 
> It says the same thing, though, so it doesn't matter.

Updated the description to refer to the living standard. The W3C spec is just a
copy of the living standard:
https://dvcs.w3.org/hg/fullscreen/raw-file/tip/Overview.html
but the living standard has been updated more recently.

[philipj_slow, 2014-07-30 12:11:18]

I think this is a sound approach...

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
File
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html
(right):

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:11:
// Bail out early if the full screen API is not enabled or is missing:
Why this case? Neither setting hides the API, they just make it fail.

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:22:
runWithKeyDown(function(){document.getElementById('video').webkitRequestFullScreen()});
It would be best to only have one webkitRequestFullScreen() call in the test.
Otherwise it's not clear which request is causing which event. If you want to
tests both in a single test then the second request should be made after the
first event has been fired.

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # The following two settings are only exposed
for Layout Tests.
That's not quite true, they're intended to be set by Android WebView...

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # FIXME: This is a temporary flag and should
be removed once
Can that ever actually happen. Won't there continue to be apps that don't
implement WebChromeClient.onShowCustomView?

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h
File public/web/WebSettings.h (right):

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h...
public/web/WebSettings.h:55: virtual bool
disallowFullscreenForNonMediaElements() const = 0;
In total there are now four functions to support this:
    virtual bool disallowFullscreenForNonMediaElements() const = 0;
    virtual bool fullscreenEnabled() const = 0;
    virtual void setDisallowFullscreenForNonMediaElements(bool) = 0;
    virtual void setFullscreenEnabled(bool) = 0;

Why two settings, and why are the first two getters needed at all? I feel like I
don't have the whole picture here...

[philipj_slow, 2014-07-30 12:17:03]

On 2014/07/30 12:10:17, Ignacio Solla wrote:
> On 2014/07/30 11:54:26, philipj wrote:
> > Since you said "W3C spec" it sounds like you've found a stable spec. This is
> the
> > one that's maintained:
> > http://fullscreen.spec.whatwg.org
> > 
> > It says the same thing, though, so it doesn't matter.
> 
> Updated the description to refer to the living standard. The W3C spec is just
a
> copy of the living standard:
> https://dvcs.w3.org/hg/fullscreen/raw-file/tip/Overview.html
> but the living standard has been updated more recently.

In this case, the W3C copy hasn't been updated since Anne took the spec to the
WHATWG. Anyway, I just wanted to point you to the most up-to-date copy in case
you didn't know about it :)

[Ignacio Solla, 2014-07-30 14:40:00]

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
File
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html
(right):

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:11:
// Bail out early if the full screen API is not enabled or is missing:
On 2014/07/30 12:11:18, philipj wrote:
> Why this case? Neither setting hides the API, they just make it fail.

Removed. Don't need it (I made a copy from another test, but I should have
removed this).

https://codereview.chromium.org/428633004/diff/40001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:22:
runWithKeyDown(function(){document.getElementById('video').webkitRequestFullScreen()});
On 2014/07/30 12:11:18, philipj wrote:
> It would be best to only have one webkitRequestFullScreen() call in the test.
> Otherwise it's not clear which request is causing which event. If you want to
> tests both in a single test then the second request should be made after the
> first event has been fired.

I've changed it to only run the second step after the first event has been
received.

I prefer to have both in one test. I think it is clearer and having this in just
one place makes it easy to understand what the behaviour should be.

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # The following two settings are only exposed
for Layout Tests.
On 2014/07/30 12:11:18, philipj wrote:
> That's not quite true, they're intended to be set by Android WebView...

Added "only exposed through Javascript".

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # FIXME: This is a temporary flag and should
be removed once
On 2014/07/30 12:11:18, philipj wrote:
> Can that ever actually happen. Won't there continue to be apps that don't
> implement WebChromeClient.onShowCustomView?

The fullscreenEnabled setting is for apps that do not implement
onShowCustomView.

The disallowFullscreenForNonMediaElements is for a limitation in the framework.
It will always be true in the WebView (well, until we add support for non-media
elements). The we will have fullscreen for non-media elements unless the app
does not implement onShowCustomView.

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h
File public/web/WebSettings.h (right):

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h...
public/web/WebSettings.h:55: virtual bool
disallowFullscreenForNonMediaElements() const = 0;
On 2014/07/30 12:11:18, philipj wrote:
> In total there are now four functions to support this:
>     virtual bool disallowFullscreenForNonMediaElements() const = 0;
>     virtual bool fullscreenEnabled() const = 0;
>     virtual void setDisallowFullscreenForNonMediaElements(bool) = 0;
>     virtual void setFullscreenEnabled(bool) = 0;
> 
> Why two settings, and why are the first two getters needed at all? I feel like
I
> don't have the whole picture here...

See my other reply that explains why we need the two settings.

We need the two getters so that we can access them from FullscreenElementStack?

[philipj_slow, 2014-07-30 15:47:18]

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # The following two settings are only exposed
for Layout Tests.
On 2014/07/30 14:40:00, Ignacio Solla wrote:
> On 2014/07/30 12:11:18, philipj wrote:
> > That's not quite true, they're intended to be set by Android WebView...
> 
> Added "only exposed through Javascript".

All settings are exposed in internals.settings for LayoutTests. These settings
seem like most, intended to be set by Chromium, it would be helpful to instead
describe what they do. It sounds like this one would disable the Fullscreen API
entirely, which isn't quite the case.

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # FIXME: This is a temporary flag and should
be removed once
On 2014/07/30 14:40:00, Ignacio Solla wrote:
> On 2014/07/30 12:11:18, philipj wrote:
> > Can that ever actually happen. Won't there continue to be apps that don't
> > implement WebChromeClient.onShowCustomView?
> 
> The fullscreenEnabled setting is for apps that do not implement
> onShowCustomView.
> 
> The disallowFullscreenForNonMediaElements is for a limitation in the
framework.
> It will always be true in the WebView (well, until we add support for
non-media
> elements). The we will have fullscreen for non-media elements unless the app
> does not implement onShowCustomView.

Let's see if I understand.

http://developer.android.com/reference/android/webkit/WebView.html says "In
order to support inline HTML5 video in your application, you need to have
hardware acceleration turned on, and set a WebChromeClient. For full screen
support, implementations of onShowCustomView(View,
WebChromeClient.CustomViewCallback) and onHideCustomView() are required,
getVideoLoadingProgressView() is optional."

In other words, if onShowCustomView isn't implemented, no fullscreen will work
at all, video or otherwise.

When onShowCustomView is implemented, non-video fullscreen still never works,
but this is something you hope to eventually fix.

Is that right? If so I understand why two settings are needed.

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h
File public/web/WebSettings.h (right):

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h...
public/web/WebSettings.h:55: virtual bool
disallowFullscreenForNonMediaElements() const = 0;
On 2014/07/30 14:40:00, Ignacio Solla wrote:
> On 2014/07/30 12:11:18, philipj wrote:
> > In total there are now four functions to support this:
> >     virtual bool disallowFullscreenForNonMediaElements() const = 0;
> >     virtual bool fullscreenEnabled() const = 0;
> >     virtual void setDisallowFullscreenForNonMediaElements(bool) = 0;
> >     virtual void setFullscreenEnabled(bool) = 0;
> > 
> > Why two settings, and why are the first two getters needed at all? I feel
like
> I
> > don't have the whole picture here...
> 
> See my other reply that explains why we need the two settings.
> 
> We need the two getters so that we can access them from
FullscreenElementStack?

I applied your patch locally and tried removing this. It works because
FullscreenElementStack uses Settings, not WebSettings.

[Ignacio Solla, 2014-07-31 10:30:51]

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # The following two settings are only exposed
for Layout Tests.
On 2014/07/30 15:47:17, philipj wrote:
> On 2014/07/30 14:40:00, Ignacio Solla wrote:
> > On 2014/07/30 12:11:18, philipj wrote:
> > > That's not quite true, they're intended to be set by Android WebView...
> > 
> > Added "only exposed through Javascript".
> 
> All settings are exposed in internals.settings for LayoutTests. These settings
> seem like most, intended to be set by Chromium, it would be helpful to instead
> describe what they do. It sounds like this one would disable the Fullscreen
API
> entirely, which isn't quite the case.

Oh, I see, thanks for the clarification!

I have added descriptions now.

https://codereview.chromium.org/428633004/diff/40001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # FIXME: This is a temporary flag and should
be removed once
On 2014/07/30 15:47:17, philipj wrote:
> On 2014/07/30 14:40:00, Ignacio Solla wrote:
> > On 2014/07/30 12:11:18, philipj wrote:
> > > Can that ever actually happen. Won't there continue to be apps that don't
> > > implement WebChromeClient.onShowCustomView?
> > 
> > The fullscreenEnabled setting is for apps that do not implement
> > onShowCustomView.
> > 
> > The disallowFullscreenForNonMediaElements is for a limitation in the
> framework.
> > It will always be true in the WebView (well, until we add support for
> non-media
> > elements). The we will have fullscreen for non-media elements unless the app
> > does not implement onShowCustomView.
> 
> Let's see if I understand.
> 
> http://developer.android.com/reference/android/webkit/WebView.html says "In
> order to support inline HTML5 video in your application, you need to have
> hardware acceleration turned on, and set a WebChromeClient. For full screen
> support, implementations of onShowCustomView(View,
> WebChromeClient.CustomViewCallback) and onHideCustomView() are required,
> getVideoLoadingProgressView() is optional."
> 
> In other words, if onShowCustomView isn't implemented, no fullscreen will work
> at all, video or otherwise.
> 
> When onShowCustomView is implemented, non-video fullscreen still never works,
> but this is something you hope to eventually fix.
> 
> Is that right? If so I understand why two settings are needed.

Exactly, your understanding is correct.

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h
File public/web/WebSettings.h (right):

https://codereview.chromium.org/428633004/diff/40001/public/web/WebSettings.h...
public/web/WebSettings.h:55: virtual bool
disallowFullscreenForNonMediaElements() const = 0;
On 2014/07/30 15:47:17, philipj wrote:
> On 2014/07/30 14:40:00, Ignacio Solla wrote:
> > On 2014/07/30 12:11:18, philipj wrote:
> > > In total there are now four functions to support this:
> > >     virtual bool disallowFullscreenForNonMediaElements() const = 0;
> > >     virtual bool fullscreenEnabled() const = 0;
> > >     virtual void setDisallowFullscreenForNonMediaElements(bool) = 0;
> > >     virtual void setFullscreenEnabled(bool) = 0;
> > > 
> > > Why two settings, and why are the first two getters needed at all? I feel
> like
> > I
> > > don't have the whole picture here...
> > 
> > See my other reply that explains why we need the two settings.
> > 
> > We need the two getters so that we can access them from
> FullscreenElementStack?
> 
> I applied your patch locally and tried removing this. It works because
> FullscreenElementStack uses Settings, not WebSettings.

That's right, sorry. Removed now.

[philipj_slow, 2014-07-31 13:15:03]

This now LGTM, thanks for taking the time to explain everything to me!

You'll need someone else to review stuff outside of Source/core, and maybe
falken has feedback as well :)

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # This value is set to false if the platform
does not support full screen.
For consistency, replace "full screen" with "fullscreen" and "fullScreen" with
"fullscreen". (I know things are really inconsistent, but I'm working on it...)

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # (onfullscreenerror or
webkitfullscreenerror) as specified in the standard:
No "on" required in "onfullscreenerror".

[falken, 2014-07-31 14:53:10]

lgtm with nits

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
File
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html
(right):

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:1:
<body>
<body> is usually omitted in layout tests. If somehow this is required, it
should have a closing </body>.

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
File LayoutTests/fullscreen/full-screen-request-not-enabled.html (right):

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:1: <body>
ditto about <body>

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:11:
logResult("Entered full screen.", false);
Seems like these arguments are switched?

function logResult(success, text)

Also, use single-quotes for consistency with rest of file.

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:18: 
nit: extra line here

[Ignacio Solla, 2014-08-01 10:02:44]

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
File
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html
(right):

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-disallow-for-non-media-elements.html:1:
<body>
On 2014/07/31 14:53:10, falken wrote:
> <body> is usually omitted in layout tests. If somehow this is required, it
> should have a closing </body>.

Done.

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
File LayoutTests/fullscreen/full-screen-request-not-enabled.html (right):

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:1: <body>
On 2014/07/31 14:53:10, falken wrote:
> ditto about <body>

Done.

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:11:
logResult("Entered full screen.", false);
On 2014/07/31 14:53:10, falken wrote:
> Seems like these arguments are switched?
> 
> function logResult(success, text)
> 
> Also, use single-quotes for consistency with rest of file.

Done.

https://codereview.chromium.org/428633004/diff/80001/LayoutTests/fullscreen/f...
LayoutTests/fullscreen/full-screen-request-not-enabled.html:18: 
On 2014/07/31 14:53:10, falken wrote:
> nit: extra line here

Done.

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
File Source/core/frame/Settings.in (right):

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
Source/core/frame/Settings.in:272: # This value is set to false if the platform
does not support full screen.
On 2014/07/31 13:15:03, philipj wrote:
> For consistency, replace "full screen" with "fullscreen" and "fullScreen" with
> "fullscreen". (I know things are really inconsistent, but I'm working on
it...)

Done.

https://codereview.chromium.org/428633004/diff/80001/Source/core/frame/Settin...
Source/core/frame/Settings.in:274: # (onfullscreenerror or
webkitfullscreenerror) as specified in the standard:
On 2014/07/31 13:15:03, philipj wrote:
> No "on" required in "onfullscreenerror".

Done.

[philipj_slow, 2014-08-01 10:39:13]

OWNERS review for web/ and this should be ready to go.

[aelias_OOO_until_Jul13, 2014-08-01 19:29:32]

lgtm for Source/web, but this still needs public/web OWNERS.  dglazkov@, could
you take a look?

[dglazkov, 2014-08-01 20:30:48]

lgtm

[philipj_slow, 2014-08-02 04:42:43]

Since this hasn't landed yet I'd suggest calling the setting fullscreenSupported
or fullscreenIsSupported to match the spec change:
http://fullscreen.spec.whatwg.org/#fullscreen-is-supported

[Ignacio Solla, 2014-08-04 08:49:54]

On 2014/08/02 04:42:43, philipj wrote:
> Since this hasn't landed yet I'd suggest calling the setting
fullscreenSupported
> or fullscreenIsSupported to match the spec change:
> http://fullscreen.spec.whatwg.org/#fullscreen-is-supported

Done.

[Ignacio Solla, 2014-08-04 08:50:44]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-08-04 08:50:55]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/428633004/120001

[commit-bot: I haz the power, 2014-08-04 09:44:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[commit-bot: I haz the power, 2014-08-04 10:44:30]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-08-04 10:44:30]

Try jobs failed on following builders:
  linux_blink_dbg on tryserver.blink
(http://build.chromium.org/p/tryserver.blink/builders/linux_blink_dbg/builds/1...)

[Ignacio Solla, 2014-08-04 11:44:42]

The CQ bit was checked by igsolla@chromium.org

[commit-bot: I haz the power, 2014-08-04 11:46:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/igsolla@chromium.org/428633004/120001

[commit-bot: I haz the power, 2014-08-04 12:44:05]

Change committed as 179470