[webkit-reviews] review denied: [Bug 85990] Volume slider needs to be displayed below the mute button : [Attachment 140931] Preliminary patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Victor Carbune
<vcarbune at adobe.com>'s request for review:
Bug 85990: Volume slider needs to be displayed below the mute button
https://bugs.webkit.org/show_bug.cgi?id=85990

Attachment 140931: Preliminary patch
https://bugs.webkit.org/attachment.cgi?id=140931&action=review

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=140931&action=review


I don't like this. We shouldn't be mutating DOM inside layout.

> Source/WebCore/css/mediaControlsChromium.css:178
> +::-webkit-media-controls *
div[class="webkit-media-controls-volume-slider-below"] {

Why do you need the * selector there?

> Source/WebCore/html/shadow/MediaControlElements.cpp:351
> +	   slider->displayBelowMuteButton(true);

Mutating DOM inside layout seems like a hideous violation of layering to me.