Intent to Deprecate and Remove: ::-webkit-media-controls* pseudo-element selectors

[Philip Jägenstedt]

Primary eng (and PM) emails

phi...@opera.com

Summary

The UI for <video controls> is implemented using shadow DOM elements, which are styled using ::-webkit-media-controls* pseudo-element selectors in the user agent stylesheet, mediaControls.css. Unfortunately, page author stylesheets can also use these selectors, overriding parts of the user agent style.

Rename ::-webkit-media-controls* to ::-internal-media-controls* and make them accessible only to the user agent stylesheet.

Motivation

In https://crbug.com/415407 (restricted, sorry) a fuzzer has found a problem where a page overrides ::-webkit-media-controls { display: ... }, hitting an assert.

https://codereview.chromium.org/662243003/ is the review, and I've been unable to find a fix which actually gets at the root of the problem other than simply unexposing the pseudo-elements to the Web. (If I had a better grip of style/layout/rendering maybe I could, who knows.)

Compatibility Risk

Unfortunately, there is some:

http://css-tricks.com/custom-controls-in-html5-video-full-screen/

This combines ::-webkit-media-controls { display: none; } with a bug in our fullscreen implementation, namely that z-index on non-fullscreen elements can allow them to appear above the fullscreen element.

Note that fixing fullscreen to use the top-layer <https://crbug.com/240576> would also break this method.

Alternative implementation suggestion for web developers

To get rid of native fullscreen controls, use a parent of the <video> element as the fullscreen element, don't use <video controls> and place things on top of the <video> as usual.

Usage information from UseCounter

None, and issue 41540 is targeted for M39 so we can't add a counter and wait. If this is unacceptable some temporary workaround in C++ will likely be needed.

Entry on chromestatus.com, crbug.com, or MDN

None.

Requesting approval to remove too?

Yes, although it's removal by renaming.

[Dimitri Glazkov]

On Mon, Oct 27, 2014 at 8:06 AM, Philip Jägenstedt <phi...@opera.com> wrote:

Primary eng (and PM) emails

phi...@opera.com

Summary

The UI for <video controls> is implemented using shadow DOM elements, which are styled using ::-webkit-media-controls* pseudo-element selectors in the user agent stylesheet, mediaControls.css. Unfortunately, page author stylesheets can also use these selectors, overriding parts of the user agent style.

Rename ::-webkit-media-controls* to ::-internal-media-controls* and make them accessible only to the user agent stylesheet.

I think we should be really careful about this. A simple query:

https://www.google.com/?#q=webkit-media-controls

shows a bunch of developer enthusiasm around these pseudoelements, and shutting the door will probably break some fingers. And then you'll be inundated with emotional emails from the injured :)

Motivation

In https://crbug.com/415407 (restricted, sorry) a fuzzer has found a problem where a page overrides ::-webkit-media-controls { display: ... }, hitting an assert.

Can you cc me on that bug? Maybe there's a solution that's less violent than finger-breaking.

[Elliott Sprehn]

It sounds like you just need a bunch of !important's in the UA sheet so the authors can't override style in unexpected ways. Also RenderMedia does sketchy stuff, we should fix it: https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/rendering/RenderMedia.cpp&q=RenderMedia.cpp&sq=package:chromium&l=62

That's blindly casting to RenderBox, but if you set the style you can make it do something bad. I suspect there's more of that kind of thing in the media code.

[Julien Chaffraix]

On Mon, Oct 27, 2014 at 11:27 AM, Elliott Sprehn <esp...@chromium.org> wrote:
> It sounds like you just need a bunch of !important's in the UA sheet so the
> authors can't override style in unexpected ways. Also RenderMedia does
> sketchy stuff, we should fix it:
> https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/rendering/RenderMedia.cpp&q=RenderMedia.cpp&sq=package:chromium&l=62
>
> That's blindly casting to RenderBox, but if you set the style you can make
> it do something bad. I suspect there's more of that kind of thing in the
> media code.

That seems orthogonal to this thread. There will be a need for a C++
solution - as Philip pointed out - but the inter-operability harm is
still there. While we should investigate another fix, we should still
try to see if we can unexposed these APIs.

>
>
> On Mon, Oct 27, 2014 at 10:36 AM, Dimitri Glazkov <dgla...@chromium.org>
> wrote:
>>
>>
>>
>> On Mon, Oct 27, 2014 at 8:06 AM, Philip Jägenstedt <phi...@opera.com>
>> wrote:
>>>
>>> Primary eng (and PM) emails
>>>
>>> phi...@opera.com
>>>
>>>
>>> Summary
>>>
>>> The UI for <video controls> is implemented using shadow DOM elements,
>>> which are styled using ::-webkit-media-controls* pseudo-element selectors in
>>> the user agent stylesheet, mediaControls.css. Unfortunately, page author
>>> stylesheets can also use these selectors, overriding parts of the user agent
>>> style.
>>>
>>>
>>> Rename ::-webkit-media-controls* to ::-internal-media-controls* and make
>>> them accessible only to the user agent stylesheet.
>>
>>
>> I think we should be really careful about this. A simple query:
>>
>> https://www.google.com/?#q=webkit-media-controls
>>
>> shows a bunch of developer enthusiasm around these pseudoelements, and
>> shutting the door will probably break some fingers. And then you'll be
>> inundated with emotional emails from the injured :)

It seems like we need better data to make this call. Without knowing
how popular these pseudos are, it's difficult to know how many fingers
we break.

Also the problem is that the more we wait, the more people will rely
on these non-standard and not meant to be exposed extensions, which is
why I am in favor of at least deprecating with the intent to un-expose
(once we have more data). Stating that they are deprecated at least
indicate our intent that you're using them at your own risk.

Julien

[Philip Jägenstedt]

On Mon, Oct 27, 2014 at 6:36 PM, Dimitri Glazkov <dgla...@chromium.org> wrote:
>
>
> On Mon, Oct 27, 2014 at 8:06 AM, Philip Jägenstedt <phi...@opera.com>
> wrote:
>>
>> Primary eng (and PM) emails
>>
>> phi...@opera.com
>>
>>
>> Summary
>>
>> The UI for <video controls> is implemented using shadow DOM elements,
>> which are styled using ::-webkit-media-controls* pseudo-element selectors in
>> the user agent stylesheet, mediaControls.css. Unfortunately, page author
>> stylesheets can also use these selectors, overriding parts of the user agent
>> style.
>>
>>
>> Rename ::-webkit-media-controls* to ::-internal-media-controls* and make
>> them accessible only to the user agent stylesheet.
>
>
> I think we should be really careful about this. A simple query:
>
> https://www.google.com/?#q=webkit-media-controls
>
> shows a bunch of developer enthusiasm around these pseudoelements, and
> shutting the door will probably break some fingers. And then you'll be
> inundated with emotional emails from the injured :)
>
>>
>> Motivation
>>
>> In https://crbug.com/415407 (restricted, sorry) a fuzzer has found a
>> problem where a page overrides ::-webkit-media-controls { display: ... },
>> hitting an assert.
>
>
> Can you cc me on that bug? Maybe there's a solution that's less violent than
> finger-breaking.

Done.

Philip

[Philip Jägenstedt]

On Mon, Oct 27, 2014 at 7:27 PM, Elliott Sprehn <esp...@chromium.org> wrote:
> It sounds like you just need a bunch of !important's in the UA sheet so the
> authors can't override style in unexpected ways.

::-webkit-media-controls { display: flex !important; } in
mediaControls.css does fix the problem, but since it is exactly the
selector and property that
http://css-tricks.com/custom-controls-in-html5-video-full-screen/
depends on, it's not actually any better than unexposing
::-webkit-media-controls completely IMHO. There are other properties
on ::-webkit-media-controls, but none that seems useful at all at
first glance.

> Also RenderMedia does sketchy stuff, we should fix it:
> https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit/Source/core/rendering/RenderMedia.cpp&q=RenderMedia.cpp&sq=package:chromium&l=62
>
> That's blindly casting to RenderBox, but if you set the style you can make
> it do something bad. I suspect there's more of that kind of thing in the
> media code.

I've investigated that particular instance and it doesn't seem
relevant to the problem. I think if it were involved the cast would
assert in debug builds. I couldn't find any other unchecked casts in
RenderMedia*, but I may have missed something.

Philip

[Philip Jägenstedt]

API OWNERS, you can ignore this thread now. With the help of Rune and
Morten, I found another solution:
https://codereview.chromium.org/689613002/

I think we should get rid of ::-webkit-media-controls*, but not today.

Philip