Bug 62218

*** Bug 62261 has been marked as a duplicate of this bug. ***

Roland, do you think this is live?

I think this is still valid, but more of a theoretical concern at the moment. Per spec, a pseudo-element selector cannot be followed by anything, except a few pseudo-classes. It's only when we relax this requirement that it may become a real issue. (setting this to P3 for now).

(In reply to comment #3) > I think this is still valid, but more of a theoretical concern at the moment. Per spec, a pseudo-element selector cannot be followed by anything, except a few pseudo-classes. It's only when we relax this requirement that it may become a real issue. > > (setting this to P3 for now). We relaxed it a while back: http://trac.webkit.org/changeset/85077.

I have an issue with nested shadow DOMs and selectors. I'm working on the video controls. They are built as a shadow dom. The transport bar and the volume slider are implemented as input[type"range"] elements. Now I want to style the input element's shadow dom. In theory, I'd like to do the following: video::-webkit-media-controls input[type="range"]::-webkit-slider-container { border: 1px solid rgb(130, 130, 130); border-radius: 5px; }

Hmm, that was incomplete. Instead of the above, I have to do: input[type="range"]::-webkit-slider-container { border: 1px solid rgb(130, 130, 130); border-radius: 5px; } otherwise I get no changes to the sliders.

(In reply to comment #5) > video::-webkit-media-controls input[type="range"]::-webkit-slider-container { A descendant combinator after a pseudo-element - I understand what you're attempting to do, but this is quite hairy from a spec point of view. Have you tried video::-webkit-media-controls::-webkit-slider-container Or even video::-webkit-slider-container ? If those don't work either, then finding a solution for something like this (even with code changes) would go over much smoother IMHO.

(In reply to comment #7) > (In reply to comment #5) > > video::-webkit-media-controls input[type="range"]::-webkit-slider-container { > > A descendant combinator after a pseudo-element - I understand what you're attempting to do, but this is quite hairy from a spec point of view. OK, I am just trying to find something that works. > Have you tried > > video::-webkit-media-controls::-webkit-slider-container This is what I tried first and it doesn't work. > Or even > > video::-webkit-slider-container Again, this doesn't work. Also, I cannot address individual sliders differently in this way. For example, I have a slider for the timeline and a slider for the volume. If I wanted to style them differently, it wouldn't be possible. (Thank god that's not currently required, but I can see it coming...) > ? If those don't work either, then finding a solution for something like this (even with code changes) would go over much smoother IMHO. I'm open for any suggestions. Other than hard coding media controls knowledge into the shadow dom of the input element, I couldn't see a solution. I don't think hard coding is the right approach.

(In reply to comment #8) > (In reply to comment #7) > > Have you tried > > > > video::-webkit-media-controls::-webkit-slider-container > > This is what I tried first and it doesn't work. Hm, I have a hunch why, but could you please post a minimal test case so that we can look at the issue in detail? > Also, I cannot address individual sliders differently in this way. For example, I have a slider for the timeline and a slider for the volume. If I wanted to style them differently, it wouldn't be possible. (Thank god that's not currently required, but I can see it coming...) Well, just use 3 levels of pseudo-elements: volume::-webkit-media-controls::-webkit-volume-control::-webkit-slider-container ... ;) (just kidding!) The problem in any case is that all of this requires more or less knowledge of the <volume>'s internal structure. The way I think we really want to take this is to use CSS Variables. You'd write video { data-webkit-volume-control-border-width: 1px; data-webkit-volume-control-border-color: rgb(130, 130, 130); data-webkit-volume-control-border-radius: 5px; } and inside the shadow DOM you'd use <style scoped> #volume::-webkit-slider-container { border: data(webkit-volume-control-border-width, 0px) solid data(webkit-volume-control-border-color, white); border-radius: data(webkit-volume-control-border-radius, 0px); } </style> or even do another stage of handing variables off: <style scoped> #volume { data-webkit-slider-border-color: data(webkit-volume-control-border-color, white); data-webkit-slider-border-width: data(webkit-volume-control-border-width, 0px); data-webkit-slider-border-radius: data(webkit-volume-control-border-radius, 0px); } </style> where the shadow DOM within <input type="range"> uses the variables similarly to set the border color and radius. But unfortunately all of this isn't ready yet. > I'm open for any suggestions. Other than hard coding media controls knowledge into the shadow dom of the input element, I couldn't see a solution. I don't think hard coding is the right approach. Actually, what is the purpose? Is this to allow the user to style the <video> element, or is it for internal styling only? In the latter case a <style scoped> inside the <video> shadow DOM might work, at least for the time being (?).

(In reply to comment #9) > > > video::-webkit-media-controls::-webkit-slider-container > > > > This is what I tried first and it doesn't work. > > Hm, I have a hunch why, but could you please post a minimal test case so that we can look at the issue in detail? The patch for Chrome that I'm working on is not landed yet, so it's kinda difficult. I'll post a link to it when I'm uploading it to the chrome bug tracker. > The problem in any case is that all of this requires more or less knowledge of the <volume>'s internal structure. The way I think we really want to take this is to use CSS Variables. You'd write > > video { > data-webkit-volume-control-border-width: 1px; > data-webkit-volume-control-border-color: rgb(130, 130, 130); > data-webkit-volume-control-border-radius: 5px; > } > where the shadow DOM within <input type="range"> uses the variables similarly to set the border color and radius. But unfortunately all of this isn't ready yet. Hmm, these are very long variable names. Also, it's not actually the control's border that I'm setting, but the first shadow child (the container) in the input control. I thought the shadowPseudoIds were a clever way of addressing the correct shadow element from CSS, but it's just unfortunate that there is only one level of hierarchical depth that we're allowed to use. > > I'm open for any suggestions. Other than hard coding media controls knowledge into the shadow dom of the input element, I couldn't see a solution. I don't think hard coding is the right approach. > > Actually, what is the purpose? Is this to allow the user to style the <video> element, or is it for internal styling only? It's for internal styling only right now. User styling is a bonus, but not strictly necessary for my Chrome video controls styling update. > In the latter case a <style scoped> inside the <video> shadow DOM might work, at least for the time being (?). The shadow dom is coded in C++. I have an external stylesheet that is applied to the shadow dom, so scoping is out of the question IIUC. If everything else fails, I think I will be able to grab the graphics context and set the CSS properties by hand. It's just that CSS is so much nicer to write than coding CSS rules in C++!

The full patch for the updated video controls is now at https://bugs.webkit.org/attachment.cgi?id=140240&action=prettypatch . I'm attaching here just the subpart that will show this bug.

Created attachment 141052 [details] Patch that exposes the bug Patch that exposes the bug

Also see https://code.google.com/p/chromium/issues/detail?id=112508

Created attachment 146812 [details] Patch

Comment on attachment 146812 [details] Patch Attachment 146812 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/12946116

Comment on attachment 146812 [details] Patch Attachment 146812 [details] did not pass mac-ews (mac): Output: http://queues.webkit.org/results/12943198

Comment on attachment 146812 [details] Patch Attachment 146812 [details] did not pass win-ews (win): Output: http://queues.webkit.org/results/12925909

Comment on attachment 146812 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=146812&action=review I think you're missing expected file, and the bots are angry. > Source/WebCore/ChangeLog:10 > + - updateSpecifiersWithElementName() didn't take nesting into account. > + tag history can contain selector entries which isn't marked as ShadowDescendant yet. > + such entry can be found by investigating isUnknownPseudoElement(). I am curious when this happens.

Created attachment 147010 [details] Patch

Hi Dimitri, thanks for taking a look. (In reply to comment #18) > (From update of attachment 146812 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=146812&action=review > > I think you're missing expected file, and the bots are angry. > > > Source/WebCore/ChangeLog:10 > > + - updateSpecifiersWithElementName() didn't take nesting into account. > > + tag history can contain selector entries which isn't marked as ShadowDescendant yet. > > + such entry can be found by investigating isUnknownPseudoElement(). > > I am curious when this happens. Exactly in this case: tag::-shadow-id::-nested-shadow-id. When the parser calls updateSpecifiersWithElementName(), there is a selector chain which looks like: [-shadow-id] <-(ShadowDescenDant) - [-nested-shadow-id] Since [-shadow-id] is the head of the chain, it has no tag history, thus no relationship to it is set. It set tag name to the selector, instead of creating new one, which creates [both tag name and -shadow-id is set] <-(ShadowDescenDant) - [-nested-shadow-id] instead of [tag] <-(ShadowDescenDant) - [-shadow-id is set] <-(ShadowDescenDant) - [-nested-shadow-id]

Created attachment 147012 [details] Patch

Comment on attachment 147012 [details] Patch Clearing flags on attachment: 147012 Committed r120147: <http://trac.webkit.org/changeset/120147>

All reviewed patches have been landed. Closing bug.