Change HTMLInputElement-derived parts of media element shadow DOM to use shadowPseudoId.
Created attachment 80115 [details] Patch
Comment on attachment 80115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80115&action=review Seems unfortunate that shadowPseudoId returns an AtomicString so has to thrash reference counts. Is there any caller that can’t return const AtomicString& instead? Might be nice to tighten that up again. > Source/WebCore/rendering/MediaControlElements.cpp:468 > + m_displayType = MediaMuteButton; Why isn’t this a MediaControlInputElement constructor argument? It seems less than optimal to set this field in a base class directly instead of letting the constructor do it.
Comment on attachment 80115 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=80115&action=review Good point on shadowPseudoId. I'll change to use const AtomicString& in separate patch. >> Source/WebCore/rendering/MediaControlElements.cpp:468 >> + m_displayType = MediaMuteButton; > > Why isn’t this a MediaControlInputElement constructor argument? It seems less than optimal to set this field in a base class directly instead of letting the constructor do it. I'll change to use a constructor argument, I like it.
Turns out I also neglected to realize that since the elements now use proper selector pipeline, some initial CSS properties need to be explicitly set to avoid inheriting values from input[type=button] and input[type=range] definitions.
Created attachment 80135 [details] With fixups
(In reply to comment #5) > Created an attachment (id=80135) [details] > With fixups Darin, can you take another look?
(In reply to comment #6) > (In reply to comment #5) > > Created an attachment (id=80135) [details] [details] > > With fixups > > Darin, can you take another look? Since you r+ the first patch, I'll land this improvement with your silent agreement :) -- please yell at me if I'd done wrong.
Comment on attachment 80135 [details] With fixups Looks good.
Committed r76719: <http://trac.webkit.org/changeset/76719>
http://trac.webkit.org/changeset/76719 might have broken Qt Linux Release
Reverted r76719 for reason: Broke a bunch of media tests in Chromium/Qt/GTK Committed r76724: <http://trac.webkit.org/changeset/76724>
(In reply to comment #11) > Reverted r76719 for reason: > > Broke a bunch of media tests in Chromium/Qt/GTK > > Committed r76724: <http://trac.webkit.org/changeset/76724> I think we may have to fix bug 52197 before proceeding further :(.
http://trac.webkit.org/changeset/76724 might have broken Leopard Intel Release (Tests)
Committed r76951: <http://trac.webkit.org/changeset/76951>
http://trac.webkit.org/changeset/76951 might have broken Qt Linux Release The following tests are not passing: media/controls-without-preload.html