RESOLVED FIXED 62218
Shadow Pseudo ID should be able to nest to point nested shadow DOM
https://bugs.webkit.org/show_bug.cgi?id=62218
Summary Shadow Pseudo ID should be able to nest to point nested shadow DOM
Dimitri Glazkov (Google)
Reported 2011-06-07 10:05:09 PDT
Per https://bugs.webkit.org/show_bug.cgi?id=52917#c15, we need to be more precise in determining whether a rule applies a given instance of the shadow DOM.
Attachments
Patch that exposes the bug (1.03 KB, text/plain)
2012-05-09 16:53 PDT, Silvia Pfeiffer
no flags
Patch (8.19 KB, patch)
2012-06-11 02:02 PDT, Hajime Morrita
no flags
Patch (13.04 KB, patch)
2012-06-11 22:50 PDT, Hajime Morrita
no flags
Patch (13.41 KB, patch)
2012-06-11 22:59 PDT, Hajime Morrita
no flags
Roland Steiner
Comment 1 2011-06-07 21:13:12 PDT
*** Bug 62261 has been marked as a duplicate of this bug. ***
Hajime Morrita
Comment 2 2012-02-27 23:52:15 PST
Roland, do you think this is live?
Roland Steiner
Comment 3 2012-02-28 00:12:42 PST
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).
Dimitri Glazkov (Google)
Comment 4 2012-02-28 08:58:27 PST
(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.
Silvia Pfeiffer
Comment 5 2012-03-14 22:16:39 PDT
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; }
Silvia Pfeiffer
Comment 6 2012-03-14 22:33:18 PDT
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.
Roland Steiner
Comment 7 2012-03-15 00:05:01 PDT
(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.
Silvia Pfeiffer
Comment 8 2012-03-15 19:23:10 PDT
(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.
Roland Steiner
Comment 9 2012-03-15 22:28:45 PDT
(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 (?).
Silvia Pfeiffer
Comment 10 2012-03-16 00:05:29 PDT
(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++!
Silvia Pfeiffer
Comment 11 2012-05-09 16:48:53 PDT
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.
Silvia Pfeiffer
Comment 12 2012-05-09 16:53:47 PDT
Created attachment 141052 [details] Patch that exposes the bug Patch that exposes the bug
Silvia Pfeiffer
Comment 13 2012-05-15 19:25:01 PDT
Hajime Morrita
Comment 14 2012-06-11 02:02:26 PDT
Gustavo Noronha (kov)
Comment 15 2012-06-11 02:13:22 PDT
Build Bot
Comment 16 2012-06-11 02:27:26 PDT
Build Bot
Comment 17 2012-06-11 02:31:17 PDT
Dimitri Glazkov (Google)
Comment 18 2012-06-11 09:03:04 PDT
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.
Hajime Morrita
Comment 19 2012-06-11 22:50:01 PDT
Hajime Morrita
Comment 20 2012-06-11 22:56:16 PDT
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]
Hajime Morrita
Comment 21 2012-06-11 22:59:59 PDT
WebKit Review Bot
Comment 22 2012-06-12 18:28:49 PDT
Comment on attachment 147012 [details] Patch Clearing flags on attachment: 147012 Committed r120147: <http://trac.webkit.org/changeset/120147>
WebKit Review Bot
Comment 23 2012-06-12 18:28:56 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.