Wilson Page [:wilsonpage] Reporter
	Description • 10 years ago

      No description provided.

	Wilson Page [:wilsonpage] Reporter
	Comment 1 • 10 years ago

In Gaia we need host-context to style shadow-dom internals based on the state of the document. This is especially important for RTL (right-to-left) when changing visual appearance of components based on document 'direction'.

:host-context([dir='rtl']) {
  /* Do RTL stuff ... */
}

	Ahmed Nefzaoui [:Nefzaoui]
	Comment 2 • 10 years ago

Since this is, of course, a blocking bug for the RTL progress.

	Ahmed Nefzaoui [:Nefzaoui]
	Comment 3 • 10 years ago

This basically prevents us from RTL'ing gaia-elements (we can still do it, but with a great part of a messy code when we could just use such feature i.e this bug, once implemented) which are slowly being widely spread accross Gaia, not to mention that RTL is a key focus for 2.2. So the result: lots of broken parts when the OS is previewed in a RTL language (e.g. Arabic).

	Julien Wajsberg [:julienw]
	Comment 4 • 10 years ago

Unless we make https://groups.google.com/d/msg/mozilla.dev.gaia/GzIIlTUKvAg/AUjnuGA8Zj8J works (I don't think we have a bug for this yet)

	Peter Bylenga [:PBylenga]
	Comment 5 • 10 years ago

Mass Edit: adding the [rtl-meta]

	bhavana bajaj [:bajaj]
	Comment 6 • 9 years ago

Blocking per RTL triage for 2.2

	Andrew Overholt [:overholt]
	Comment 7 • 9 years ago

William's working on this.

	bhavana bajaj [:bajaj]
	Comment 8 • 9 years ago

Thanks Andrew!

In-terms of timeline 2.2 has the Feature landing milestone on Feb 23, which is when we want to wrap up all the P1 RTL work. Please let us know if this is something that is risky to meet that timeline so we can work it something out.

	William Chen [:wchen]
	Comment 10 • 9 years ago

(In reply to bhavana bajaj [:bajaj] from comment #8)
> Thanks Andrew!
> 
> In-terms of timeline 2.2 has the Feature landing milestone on Feb 23, which
> is when we want to wrap up all the P1 RTL work. Please let us know if this
> is something that is risky to meet that timeline so we can work it something
> out.

I've been told that we have a work around for this bug.

	Dylan Oliver [:doliver]
	Comment 11 • 9 years ago

(In reply to William Chen [:wchen] from comment #10)
> I've been told that we have a work around for this bug.

Can we get a confirmation here and unblock 2.2 if this is no longer required?

	Andrew Overholt [:overholt]
	Comment 12 • 9 years ago

(In reply to Dylan Oliver [:doliver] from comment #11)
> (In reply to William Chen [:wchen] from comment #10)
> > I've been told that we have a work around for this bug.
> 
> Can we get a confirmation here and unblock 2.2 if this is no longer required?

Wilson is probably the best person to provide that confirmation but it looks like he's out until the 16th.  Kevin/Etienne, do either of you know if we have a workaround that avoids using the :host-context() selector?

Thanks.

	Kevin Grandon :kgrandon
	Comment 13 • 9 years ago

I think for normal usage we are ok, but I don't believe we have a workaround for RTL and web components yet. This was deemed blocking based on RTL needs and web components. 

I'm going to ask Ahmed here as he knows RTL quite well. Ahmed, are you aware of any RTL issues with components that are currently blocked by this for 2.2?

	Julien Wajsberg [:julienw]
	Comment 14 • 9 years ago

I think we have a "directionality change" event now, that we can use (but I can't find the source right now, nor the right name).

	Andrew Overholt [:overholt]
	Comment 15 • 9 years ago

I'm also going to leave a hanging needinfo here for Wilson when he returns.

	William Chen [:wchen]
	Comment 16 • 9 years ago

We need to keep track of ancestors that are shadow root hosts because we need to know when to walk shadow root rules for descendants so selectors like this will work:

::host-context(.foo) > .bar

	William Chen [:wchen]
	Comment 17 • 9 years ago

Since ::host-context pierces shadow root boundaries, some content outside of a shadow root will need to walk the rules in the shadow root, looking for ::host-context. This needs to happen when the element is a shadow root host, or if it's a descendant of such an element.

	William Chen [:wchen]
	Comment 18 • 9 years ago

The spec mentions that for the host pseudo class selectors, "when evaluated in the context of a shadow tree, matches the shadow tree’s host element. In any other context, it matches nothing" [1]

I want *:host-context(.foo) to match nothing and :host-context(.foo) to match a host with class foo, but it seems to me like these selectors are indistinguishable right now so I would like to add a way to determine if a selector is featureless.

[1] http://drafts.csswg.org/css-scoping/#host-selector

	William Chen [:wchen]
	Comment 19 • 9 years ago

Matching the selector as specified by http://drafts.csswg.org/css-scoping/#selectordef-host-context

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 22 • 9 years ago

Comment on attachment 8578099 [details] [diff] [review]
Part 3: Make it possible to determine if style rules have explicitly specified features.

(In reply to William Chen [:wchen] from comment #18)
> I want *:host-context(.foo) to match nothing and :host-context(.foo) to
> match a host with class foo, but it seems to me like these selectors are
> indistinguishable right now so I would like to add a way to determine if a
> selector is featureless.

I don't think that's what the text you quoted from the spec means; it's certainly not how selectors work in general, and I don't think anybody in the working group thinks they should work that way.

I'm not sure what the text does mean, though.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 23 • 9 years ago

from https://log.csswg.org/irc.w3.org/css/2015-03-16/#e533427 :

[2015-03-16 10:39:48 -0700] <dbaron> TabAtkins, does http://drafts.csswg.org/css-scoping/ define what it means for a selector to be evaluated in the context of a shadow tree?
[2015-03-16 10:49:24 -0700] <TabAtkins> Not well! I wasn't sure what the right hooks were. But it should be "from a <style> or <link> inside a shadow tree, or in a DOM api (somehow) rooted in a shadow tree".
[2015-03-16 10:49:34 -0700] <TabAtkins> Dunno how to write the last part.

	Wilson Page [:wilsonpage] Reporter
	Comment 24 • 9 years ago

(In reply to Julien Wajsberg [:julienw] (PTO March 7th -> 15th) from comment #14)
> I think we have a "directionality change" event now, that we can use (but I
> can't find the source right now, nor the right name).

We have this temporary mutation-observer backed solution/hack [1] so that components can react when the language changes. So we're not really blocked, but it is hacky. We need :host-context or :dir() working in shadow-dom to enable more robust solutions.

[1] https://github.com/gaia-components/gaia-component/blob/master/gaia-component.js#L362

	Wilson Page [:wilsonpage] Reporter
	Comment 25 • 9 years ago

You can checkout gaia-switch [1] as an example of this workaround in action.

[1] https://github.com/gaia-components/gaia-switch/blob/master/gaia-switch.js#L57

	Julien Wajsberg [:julienw]
	Comment 26 • 9 years ago

I see there is a "languagechange" event on "document" that's being used in l10n.js.

	Cameron McCormack (:heycam)
	Comment 27 • 9 years ago

(In reply to Wilson Page [:wilsonpage] (PTO until 16th) from comment #1)
> :host-context([dir='rtl']) {
>   /* Do RTL stuff ... */
> }

This won't work if you have an intervening dir="ltr" attribute, or if you use dir="auto".  How about instead using :-moz-dir:

  :host(:-moz-dir(rtl)) {
  }

Are there any problems with :-moz-dir() state propagating into shadow trees?  If not, then you wouldn't need to select up into the light tree at all.

	Cameron McCormack (:heycam)
	Comment 28 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #27)
> If not, then you wouldn't need to select up into the light tree at all.

That is, you could just write

  span:-moz-dir(rtl) {
  }

in your shadow tree's style sheet.

	Cameron McCormack (:heycam)
	Comment 29 • 9 years ago

(In reply to David Baron [:dbaron] (UTC-7) from comment #22)
> I don't think that's what the text you quoted from the spec means; it's
> certainly not how selectors work in general, and I don't think anybody in
> the working group thinks they should work that way.
> 
> I'm not sure what the text does mean, though.

I'm assuming that :host means "matches an element that has a shadow tree in which the element being styled resides".

Is it really desirable to be able to style the host element from within the shadow tree's style sheet?  That seems a bit odd, and would be allowed if you just wrote:

  :host { ... }

Using :host to jump out from an element in a shadow tree to the host is strange, given it's a pseudo-class.  These things don't work like other pseudo-classes do (i.e., just adding an additional check on a selector).  They also the meaning of the following combinator.  For example with:

  :host-context(.foo) > .bar { ... }

the ">" now doesn't mean a normal ">", but a shadow-piercing ">".

I think you can write an equivalent rule like this:

  .foo >>> :host::shadow > .bar,
  .foo:host::shadow > .bar { ... }

The css-scoping spec does talk about using :host::shadow from within the shadow tree, so that should work.  If you can write the equivalent of what :host-context() is trying to do using ">>>", ":host" and "::shadow", then I think that would be preferable.

And given you need to use ::shadow to jump from the element in the shadow tree to the host element, a lone :host without a ::shadow on it would not select anything and thus you wouldn't be able to style the host element from within the shadow tree.  Which I think is a good thing.

Alternatively, we could have a pseudo-class that matches not the host element, but whether a given element is in a shadow tree and has a matching host element.  So:

  .bar:hosted(.foo)

and:

  .bar:hosted-context(.foo)

Now it is more like an :any() pseudo-class.

	Cameron McCormack (:heycam)
	Comment 30 • 9 years ago

I'm going to hold off on review until it's clearer how we want these to behave.

	Julien Wajsberg [:julienw]
	Comment 31 • 9 years ago

See bug 1100912 for the issue of :dir propagating to shadow DOM.

	Andrew Overholt [:overholt]
	Comment 32 • 9 years ago

(In reply to Wilson Page [:wilsonpage] from comment #24)
> (In reply to Julien Wajsberg [:julienw] (PTO March 7th -> 15th) from comment
> #14)
> > I think we have a "directionality change" event now, that we can use (but I
> > can't find the source right now, nor the right name).
> 
> We have this temporary mutation-observer backed solution/hack [1] so that
> components can react when the language changes. So we're not really blocked,
> but it is hacky.

Given this ^ and the amount of work remaining to be done on this selector, I think we should unblock for B2G 2.2.

	William Chen [:wchen]
	Comment 33 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #29)
> (In reply to David Baron [:dbaron] (UTC-7) from comment #22)
> > I don't think that's what the text you quoted from the spec means; it's
> > certainly not how selectors work in general, and I don't think anybody in
> > the working group thinks they should work that way.
> > 
> > I'm not sure what the text does mean, though.
> 
> I'm assuming that :host means "matches an element that has a shadow tree in
> which the element being styled resides".
> 
After checking with one of the editors, it appears that this isn't the meaning. The :host-selector will only match if it has nothing to the left because in shadow tree styles, the host element is featureless and :host-selector will fail to match against non-host elements.

> Using :host to jump out from an element in a shadow tree to the host is
> strange, given it's a pseudo-class.  These things don't work like other
> pseudo-classes do (i.e., just adding an additional check on a selector). 
> They also the meaning of the following combinator.  For example with:
> 
>   :host-context(.foo) > .bar { ... }
> 
> the ">" now doesn't mean a normal ">", but a shadow-piercing ">".
> 
I didn't see anything that would change the meaning of ">", the way I implemented it in the patch is that ">" would descend to the children of the host because :host-context matches the host element.
> I think you can write an equivalent rule like this:
> 
>   .foo >>> :host::shadow > .bar,
>   .foo:host::shadow > .bar { ... }
> 
Due to the quirk I mentioned above, those selectors wouldn't match anything.

	Cameron McCormack (:heycam)
	Comment 34 • 9 years ago

(In reply to William Chen [:wchen] from comment #33)
> After checking with one of the editors, it appears that this isn't the
> meaning. The :host-selector will only match if it has nothing to the left
> because in shadow tree styles, the host element is featureless and
> :host-selector will fail to match against non-host elements.

So the host element appears different to the shadow tree.  And I'm guessing that because it's featureless that is how *:host can be different from :host()?  This concept of an element being exposed to selectors as featureless is new (which is why it seems strange).

Just to clarify, the host element is the one in the light tree, which has the shadow tree hanging off it, yes?  Unlike the (only) shadow-piercing combinator (">>>"), the regular " " and ">" combinators only look at normal DOM parent/child relationships.  And in the context of an element in the shadow tree, you can't walk parentNode to get to the host element.

However we do have the ShadowRoot node available, and the ShadowRoot is kind of featureless, so I could well believe that "*" doesn't match it since a ShadowRoot doesn't have a tag name.  Could it be that :host() matches the ShadowRoot node and that its selector argument matches against that ShadowRoot's host?  This means we don't have to squint and read ">" after a :host() as "traverse the shadow tree boundary".  Does this interpretation make sense to you?

Side question: does/should :root match anything in the shadow tree?

	William Chen [:wchen]
	Comment 35 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #34)
> So the host element appears different to the shadow tree.  And I'm guessing
> that because it's featureless that is how *:host can be different from
> :host()?  This concept of an element being exposed to selectors as
> featureless is new (which is why it seems strange).
Yes
> Just to clarify, the host element is the one in the light tree, which has
> the shadow tree hanging off it, yes?  Unlike the (only) shadow-piercing
> combinator (">>>"), the regular " " and ">" combinators only look at normal
> DOM parent/child relationships.  And in the context of an element in the
> shadow tree, you can't walk parentNode to get to the host element.
Yes, the host element is the one in the light tree, and you can not walk parentNode to get out of the shadow tree.
> However we do have the ShadowRoot node available, and the ShadowRoot is kind
> of featureless, so I could well believe that "*" doesn't match it since a
> ShadowRoot doesn't have a tag name.  Could it be that :host() matches the
> ShadowRoot node and that its selector argument matches against that
> ShadowRoot's host?  This means we don't have to squint and read ">" after a
> :host() as "traverse the shadow tree boundary".  Does this interpretation
> make sense to you?
I'm not sure it makes sense to match :host against the ShadowRoot because it's not rendered and doesn't get a box that we can style. I also don't think there is anything too weird about child combinators after a :host pseudo class. For example, for the selector :host > .foo, if I read it from left to right, I would first match the host element, then I would match against its children that have the foo class. The ">" doesn't traverse the boundary, the :host pseudo class does.
> Side question: does/should :root match anything in the shadow tree?
In Gecko right now, no. According to the spec, it would normally match <html>.

	Cameron McCormack (:heycam)
	Comment 36 • 9 years ago

(In reply to William Chen [:wchen] from comment #35)
> I'm not sure it makes sense to match :host against the ShadowRoot because
> it's not rendered and doesn't get a box that we can style.

I think that's OK since we will never resolve styles for a ShadowRoot.

> I also don't
> think there is anything too weird about child combinators after a :host
> pseudo class. For example, for the selector :host > .foo, if I read it from
> left to right, I would first match the host element, then I would match
> against its children that have the foo class. The ">" doesn't traverse the
> boundary, the :host pseudo class does.

I think having a pseudo-class traverse the shadow boundary is strange. :-)

In any case, I think the only practical difference between describing :host() in terms of matching the ShadowRoot and describing it in terms of matching the host element and affecting the following combinator is whether the host element itself can be styled from style sheets within the shadow tree.  (And whether you can do querySelector(":host") within the shadow tree to get the host element.)  Do you know if there are use cases for doing that?  Or whether this works in Chrome (where I'm assuming this is implemented already)?

Looking at the spec again, it's not actually clear to me that :host-context() is meant to select the host element, or rather whether it's just a condition to check on existing element (whether it is an element in the specified host context, i.e. like :matches() / :-moz-any()).

	William Chen [:wchen]
	Comment 37 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #36)
> In any case, I think the only practical difference between describing
> :host() in terms of matching the ShadowRoot and describing it in terms of
> matching the host element and affecting the following combinator is whether
> the host element itself can be styled from style sheets within the shadow
> tree.  (And whether you can do querySelector(":host") within the shadow tree
> to get the host element.)  Do you know if there are use cases for doing
> that?  Or whether this works in Chrome (where I'm assuming this is
> implemented already)?
Yes, we do have use cases for styling the host element itself. If you search for :host in Gaia, you can see some examples of how it's being used (in Gaia they have a hack to inline :host rules to a style sheet in the document). In Chrome, :host, :host() and :host-context() can all be used to style the host element. querySelector(":host") in the shadow tree doesn't seem to return anything but I'm not really sure how we want that to work.
> Looking at the spec again, it's not actually clear to me that
> :host-context() is meant to select the host element, or rather whether it's
> just a condition to check on existing element (whether it is an element in
> the specified host context, i.e. like :matches() / :-moz-any()).
When I asked about it, it was pretty explicit that having anything to the left of :host in the selector will cause it to fail matching. It's also how it behaves in Chrome right now.

	Cameron McCormack (:heycam)
	Comment 38 • 9 years ago

OK.  So I guess I still have some reservations about this, and how well it fits in to the model of CSS selectors, which I'll raise on www-style.  But for the moment, to get this Q1 goal working for Gaia apps, we can go ahead with this as long it's behind the Web Components pref.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 39 • 9 years ago

Comment on attachment 8578099 [details] [diff] [review]
Part 3: Make it possible to determine if style rules have explicitly specified features.

>+bool nsCSSSelector::HasExplicitFeatures()

Please call this HasFeatureSelectors() to match the spec's terminology,
and make the method const.

>+  return mExplicitUniversal || mLowercaseTag || mCasedTag ||
>+    mIDList || mClassList || mAttrList;

You don't need to check both mLowercaseTag and mCasedTag.  (They're also
slightly different in the case of pseudo-elements; see IsPseudoElement()
.) Instead of *both*, please call HasTagSelector().

Finally, please line up mIDList (or whatever ends up there with wrapping
at less than 80 characters after the above change) with mExplicitUniversal.

>+void nsCSSSelector::SetHasExplicitUniversal()
>+{
>+  mExplicitUniversal = true;
>+}

Please move the definition to inline within the header file.

>+  bool            mExplicitUniversal; // True if universal selector explicitly
>+                                      // appears in the selector.

Please put this next to mOperator, and write a separate patch to
change mOperator from char16_t to char, so that it doesn't increase
the size of nsCSSSelector.

You also need to change the code that serializes selectors in
nsCSSSelector::AppendToStringWithoutCombinatorsOrNegations so that this
new distinction round-trips correctly through serialization and
reparsing.  Those changes should have tests.

r=dbaron on this patch, but it shouldn't land without those two
additional patches mentioned in the previous two comments

	bhavana bajaj [:bajaj]
	Comment 40 • 9 years ago

(In reply to Andrew Overholt [:overholt] from comment #32)
> (In reply to Wilson Page [:wilsonpage] from comment #24)
> > (In reply to Julien Wajsberg [:julienw] (PTO March 7th -> 15th) from comment
> > #14)
> > > I think we have a "directionality change" event now, that we can use (but I
> > > can't find the source right now, nor the right name).
> > 
> > We have this temporary mutation-observer backed solution/hack [1] so that
> > components can react when the language changes. So we're not really blocked,
> > but it is hacky.
> 
> Given this ^ and the amount of work remaining to be done on this selector, I
> think we should unblock for B2G 2.2.

Agree, removing the nom here given we have a work around and this may be a bigger change to land on 2.2 at this point.

	Cameron McCormack (:heycam)
	Comment 41 • 9 years ago

Comment on attachment 8578096 [details] [diff] [review]
Part 2: Walk relevant rule in shadow roots rule processors that may have participating :host-context selectors.

Review of attachment 8578096 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xbl/nsBindingManager.cpp
@@ +701,5 @@
> +  // may be and there may be rules in an ancestor's shadow root style sheets that
> +  // apply to aData->mElement.
> +  if (!currentShadow && !aData->mTreeMatchContext.mShadowHosts.IsEmpty()) {
> +    currentShadow = aData->mTreeMatchContext.mShadowHosts.LastElement()->GetShadowRoot();
> +  }

Let me clarify the behaviour here.  Say we have this document tree:

  <html>
    <body>
      <div>
        (ShadowRoot)
          <style>:host span { color: blue; }</style>
          <content></content>
        <span>hello</span>

and aData->mElement is the span.  At this point, mTreeMatchContext.mShadowHosts should have the div as its top element.  We will then walk the :host rules from the div's shadow tree, and thus walk the |:host span| rule.

Is that really what we want?  Shouldn't the only way to style the span be to have a rule in the outer document, or to have a ::content rule in the shadow tree?  I would have thought that the only element outside a given shadow tree that can be styled by a :host rule in the shadow tree is the host element.

@@ +769,5 @@
> +namespace {
> +
> +struct EnumRuleProcessorsData {
> +  nsAutoPtr<RuleProcessorSet>* mSet;
> +  bool mOnlyWalkShadowHosts;

"mOnlyShadowRootRuleProcessors"?  It's more of an instruction to EnumRuleProcessors, than anything to do with rule walking.

@@ +793,5 @@
> +  // for its immediate binding, and one more for each binding in the
> +  // inheritance chain. Additionally, a bound content may host multiple
> +  // shadow roots, each with its own rule processor.
> +  nsXBLBinding *binding = boundContent->GetXBLBinding();
> +  while (binding) {

While touching the lines in this hunk, move the "*"s next to their types.

@@ +855,5 @@
>    set->EnumerateEntries(EnumWalkAllRules, &data);
>  }
>  
> +void
> +nsBindingManager::WalkAllShadowRootRules(nsIStyleRuleProcessor::EnumFunc aFunc,

How about "WalkShadowRootHostRules"?

::: dom/xbl/nsBindingManager.h
@@ +112,5 @@
>                       bool* aCutOffInheritance);
>  
>    void WalkAllRules(nsIStyleRuleProcessor::EnumFunc aFunc,
> +                    ElementDependentRuleProcessorData* aData,
> +                    bool aOnlyWalkShadowRootRules = false);

Having "All" in the method name but then having an argument to say "well not quite all" isn't great, but I don't have a better suggestion currently.  Also I think it'd be better not to have this argument optional.

::: layout/style/nsStyleSet.cpp
@@ +1207,5 @@
>      (*aFunc)(mRuleProcessors[eSVGAttrAnimationSheet], aData);
>  
>    bool cutOffInheritance = false;
>    if (mBindingManager) {
> +    mBindingManager->WalkAllShadowRootRules(aFunc, aData);

Why is this call needed?  Won't the WalkAllRules/WalkRules calls just below end up walking the :host rules from the shadow tree too?

	William Chen [:wchen]
	Comment 42 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #41)
> Comment on attachment 8578096 [details] [diff] [review]
> 
> Is that really what we want?  Shouldn't the only way to style the span be to
> have a rule in the outer document, or to have a ::content rule in the shadow
> tree?  I would have thought that the only element outside a given shadow
> tree that can be styled by a :host rule in the shadow tree is the host
> element.

The only use case I've seen for the host selectors is to style the host element itself. I'm not sure if we really want the behavior you've pointed out but it just seems to be how thing will work unless we find another way to interpret how child combinators should work in the presence of a host selector.

I don't feel that the behavior is too strange because children of a shadow host belong to the shadow tree in the sense that it decides whether they should be rendered and where to be rendered. Although, it does seem to overlap with the ::content selector since you could just use ::content to style any relevant children of the host.

> Why is this call needed?  Won't the WalkAllRules/WalkRules calls just below
> end up walking the :host rules from the shadow tree too?

When we make a dynamic change to an ancestor of the host element that could affect :host-context matching, we need to walk the rules from the shadow tree of any potentially affected descendants so that the RestyleManager can discover the dependency. WalkRule will currently only walk rules if the element is the host or a descendant of the host since those are the only elements that can be styled by the host rules. WalkAllRules would work too, but the callers currently pass in aWalkAllXBLStylesheets = false. XBL doesn't have the ability to style stuff outside the shadow tree, so instead of walking all XBL style sheets, I added a method to only walk shadow root sheets.

	Cameron McCormack (:heycam)
	Comment 43 • 9 years ago

(In reply to William Chen [:wchen] from comment #42)
> The only use case I've seen for the host selectors is to style the host
> element itself. I'm not sure if we really want the behavior you've pointed
> out but it just seems to be how thing will work unless we find another way
> to interpret how child combinators should work in the presence of a host
> selector.
> 
> I don't feel that the behavior is too strange because children of a shadow
> host belong to the shadow tree in the sense that it decides whether they
> should be rendered and where to be rendered. Although, it does seem to
> overlap with the ::content selector since you could just use ::content to
> style any relevant children of the host.

Reading http://dev.w3.org/csswg/css-scoping/#selectors-data-model we explicitly don't want to allow span to be matched here, based on what the "selector match list" is defined to be: the host element, all of the top-level elements in the host element's shadow tree, and all of those top-level elements regular DOM descendants.

> When we make a dynamic change to an ancestor of the host element that could
> affect :host-context matching, we need to walk the rules from the shadow
> tree of any potentially affected descendants so that the RestyleManager can
> discover the dependency. WalkRule will currently only walk rules if the
> element is the host or a descendant of the host since those are the only
> elements that can be styled by the host rules. WalkAllRules would work too,
> but the callers currently pass in aWalkAllXBLStylesheets = false. XBL
> doesn't have the ability to style stuff outside the shadow tree, so instead
> of walking all XBL style sheets, I added a method to only walk shadow root
> sheets.

Does this mean we can move the WalkAllShadowRootRules call into the else branch of the following if statement?

	William Chen [:wchen]
	Comment 44 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #43)
> Reading http://dev.w3.org/csswg/css-scoping/#selectors-data-model we
> explicitly don't want to allow span to be matched here, based on what the
> "selector match list" is defined to be: the host element, all of the
> top-level elements in the host element's shadow tree, and all of those
> top-level elements regular DOM descendants.
I think you're right, so I can remove the code in WalkRules that checks if an ancestor is a shadow root host. I played around with using child combinators in Chrome and it seems like they do make the combinators cross the shadow root boundary, I guess it's specified by this statement: "For the purpose of Selectors, a host element also appears in each of its shadow trees, with the contents of the shadow tree treated as its children." I'll attach another patch for this.
> Does this mean we can move the WalkAllShadowRootRules call into the else
> branch of the following if statement?
Yes it does.

	Cameron McCormack (:heycam)
	Comment 46 • 9 years ago

Comment on attachment 8584076 [details] [diff] [review]
Part 2.1: Make shadow root host behave as a featureless parent of shadow root's top-level elements for shadow root selector rules.

Review of attachment 8584076 [details] [diff] [review]:
-----------------------------------------------------------------

::: dom/xbl/nsBindingManager.cpp
@@ +727,5 @@
> +      aData->mTreeMatchContext.mScopedRoot = aData->mElement;
> +      associatedBinding->WalkRules(aFunc, aData);
> +    }
> +  }
> +

Per discussion in IRC, I think walking the :host rules here, and the non-:host rules later, will result in rules being walked in the wrong order.  It would mean that any rule with a :host in it would be treated as if it had lower specificity than rules that didn't.

So what we will need to do is have the part 4 patch walk :host rules even when mOnlyMatchHostPseudo==false.  We could do this either by having a check at the top of the function that if mOnlyMatchHostPseudo==true and we look at the final link of the selector and don't find a :host, then we return false.  Or we could just set a boolean when we do encounter a :host during the normal processing of the selector, and then at the very end return false if mOnlyMatchHostPseudo==true and we didn't encounter the :host.

	Cameron McCormack (:heycam)
	Comment 47 • 9 years ago

Comment on attachment 8578096 [details] [diff] [review]
Part 2: Walk relevant rule in shadow roots rule processors that may have participating :host-context selectors.

Awaiting a new version of this patch.

	William Chen [:wchen]
	Comment 48 • 9 years ago

(In reply to Cameron McCormack (:heycam) from comment #46)
> So what we will need to do is have the part 4 patch walk :host rules even
> when mOnlyMatchHostPseudo==false.  We could do this either by having a check
> at the top of the function that if mOnlyMatchHostPseudo==true and we look at
> the final link of the selector and don't find a :host, then we return false.
> Or we could just set a boolean when we do encounter a :host during the
> normal processing of the selector, and then at the very end return false if
> mOnlyMatchHostPseudo==true and we didn't encounter the :host.

I got rid of the part where we walk :host rules and normal rules separately. Part 4 already walks :host rules even when mOnlyMatchHostPseudo == false (using the first approach you suggested), it just doesn't match any :host rules in SelectorMatches. Since we now want to match regardless of whether mOnlyMatchHostPsuedo is true or false, I have changed :host matching in SelectorMatches to instead check whether the host element matches mTreeMatchContext.mScopedRoot since it is set to the host before walking the rules in the shadow tree (this change will be reflected in an updated version of part 4).

	Cameron McCormack (:heycam)
	Comment 49 • 9 years ago

(In reply to William Chen [:wchen] from comment #48)
> Since we now want to match regardless of whether
> mOnlyMatchHostPsuedo is true or false, I have changed :host matching in
> SelectorMatches to instead check whether the host element matches
> mTreeMatchContext.mScopedRoot since it is set to the host before walking the
> rules in the shadow tree (this change will be reflected in an updated
> version of part 4).

Yeah that sounds like the right thing to do.

	William Chen [:wchen]
	Comment 50 • 9 years ago

I've made all the suggested changes mentioned in the previous comments.

	Wilson Page [:wilsonpage] Reporter
	Comment 51 • 9 years ago

william: Can you confirm what new selectors will we have in the platform nce this bug is resolved? I want to make sure we don't break anything in Gaia and have everything prepared. As we're currently doing some funky polyfills and workarounds :)

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 52 • 9 years ago

Comment on attachment 8578100 [details] [diff] [review]
Part 4: Implement matching for :host-context selector

Please link to the canonical URL at http://dev.w3.org/csswg/css-scoping/
rather than to the drafts.csswg.org URL (in nsCSSPseudoClassList.h).


It seems like this selector should be behind a pref.  It seems odd to
ship without :host and :host(), and also seems like we should ship it
at the same time as shadow DOM (have we shipped that?).


So, the first question is whether our existing code for :-moz-any(),
which really predates any useful spec, matches the current spec for
:host-context().

http://dev.w3.org/csswg/css-scoping/#selectordef-host-context links to
http://dev.w3.org/csswg/selectors-4/#typedef-compound-selector which
says it should be a selector list of *compound selectors*.

That actually matches our current implementation, which is good,
with the possible exception of our current implementation rejecting
selectors that have pseudo-elements.  So I think that's fine code-wise,
unless I get an unexpected answer to:
https://lists.w3.org/Archives/Public/www-style/2015Mar/0422.html


Maybe rename AnySelectorInListMatches to AnySelectorInArgListMatches
to make it clearer that it's to be used for selector lists that are
arguments to other selectors?


>+          // In order to match host-context, the element must be a shadow root host,
>+          // we must be matching only against host pseudo selectors, and the
>+          // selector's context must be the shadow root (the selector must be featureless,
>+          // the left-most selector, and be in a shadow root style).
>+          // If the UNKNOWN selector flag is set, relax the shadow root host requirement
>+          // because this pseudo class walks through ancestors looking for a match, thus
>+          // the selector can be dependant on aElement even though it is not the host. The
>+          // dependency would otherwise be missed because when UNKNOWN is set, selector
>+          // matching may not have started from the top.

First, please wrap comments at less than 80 characters (preferably 72).


Second, though, the idea that the host element is featureless isn't
specific to the :host-context() selector.  It's described in
http://dev.w3.org/csswg/css-scoping/#host-element-in-tree , and it
seems like you need to implement it by preventing such elements from
matching all feature selectors, rather than doing something specific
for :host-context().  That is, you've written code that prevents
"p:host-context(p)" from matching, but you still haven't done anything
to prevent "p" from matching.

It feels like these two things should, actually, be two entirely separate
patches (maybe even separate bugs).

(This separate patch probably requires changes to both SelectorMatchesTree
and SelectorMatches.)


That said, I think you've written parts of that patch here, mixed in
with the implementation of :host-context().  I'm skeptical of the code
being in ContentEnumFunc and AttributeEnumFunc rather than
SelectorMatchesTree (although care needs to be taken with
pseudo-elements).

I also don't see where you're setting mOnlyMatchHostPseudo.

But maybe I should be reviewing the updated part 4 mentioned in
comment 49 instead, and that would make more sense?  More explanation
of what you're intending to do here would also help.

	William Chen [:wchen]
	Comment 53 • 9 years ago

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #52)
> Comment on attachment 8578100 [details] [diff] [review]
> Part 4: Implement matching for :host-context selector
> 
> It seems like this selector should be behind a pref.  It seems odd to
> ship without :host and :host(), and also seems like we should ship it
> at the same time as shadow DOM (have we shipped that?).
There is another bug with patches for for :host/:host() (Bug 992245) but it builds on things I've added in this bug. I've put the selector behind the same pref as shadow DOM (no, we haven't shipped to the web).
> It feels like these two things should, actually, be two entirely separate
> patches (maybe even separate bugs).
> 
> (This separate patch probably requires changes to both SelectorMatchesTree
> and SelectorMatches.)
I've made some changes to make featureless elements less :host-context specific and split it out into another patch.
> That said, I think you've written parts of that patch here, mixed in
> with the implementation of :host-context().  I'm skeptical of the code
> being in ContentEnumFunc and AttributeEnumFunc rather than
> SelectorMatchesTree (although care needs to be taken with
> pseudo-elements).
> 
> I also don't see where you're setting mOnlyMatchHostPseudo.
> 
> But maybe I should be reviewing the updated part 4 mentioned in
> comment 49 instead, and that would make more sense?  More explanation
> of what you're intending to do here would also help.
The code in SelectorMatches for host-context matching is what is specified in the spec. mOnlyMatchHostPseudo is set in nsBindingManager (added in the part 2 patch). The host isn't in the shadow tree, but needs to walk the rules from the shadow tree in order to look for host pseudo class rules. At the same time, it must ignore all other rules because they do not apply to the host. The purpose of mOnlyMatchHostPseudo and the code in ContentEnumFunc and AttributeEnumFunc is to skip all non-applicable rules when the host is walking rules in the shadow tree.

	William Chen [:wchen]
	Comment 54 • 9 years ago

In this patch I added a new member to NodeMatchContext to indicate whether the node should be treated as a featureless element. In the case that it is, we only allow featureless selectors that contain the special pseudo-class to match (the pseudo-class check is added in part 4). There are currently 2 places where we set the featureless flag on NodeMatchContext; in nsBindingManager when walking :host-context rules for shadow root host, and in SelectorMatchesTree when stepping out of a shadow root to the shadow root host when matching against a selector with a descendant combinator (this code to walk out of a shadow root was added in part 2.1).

	William Chen [:wchen]
	Comment 55 • 9 years ago

Patch as requested in comment 39.

	William Chen [:wchen]
	Comment 56 • 9 years ago

Patch as requested in comment 39

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 57 • 9 years ago

I'm sort of stuck on the reviews here because I don't think the patches match the spec, but I also don't think the spec makes sense:
https://lists.w3.org/Archives/Public/www-style/2015Apr/0061.html

It might help me move forward if you can explain what it is you're trying to implement, and also whether that matches what Chrome does.  Knowing what you're intending to implement would let me evaluate both (a) whether that makes sense and (b) whether it's implemented correctly; trying to figure out both (a) and (b) without knowing the intent that's in the middle is hard.

	William Chen [:wchen]
	Comment 58 • 9 years ago

The behavior I'm trying to implement is to make sure that the host element doesn't match selectors other than :host, :host() and :host-context. Chrome maybe does the same thing, at least it doesn't match selectors you mentioned in the mailing list post (:hover and :not(:hover)).

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 59 • 9 years ago

Comment on attachment 8586495 [details] [diff] [review]
Part 3.1: Change type of nsCSSSelector::mOperator from char16_t to char.

You should also remove the explicit casts of constants to char16_t when comparing against or assigning to mOperator; there's one in StyleRule.cpp and three in nsCSSRuleProcessor.cpp.  (The ones that are char16_t(0) could either become char(0) or '\0'; I think char(0) is probably clearer.)

r=dbaron with that

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 60 • 9 years ago

Comment on attachment 8586496 [details] [diff] [review]
Part 3.2: Update selector serialization to preserve featureless selectors.

>-    if (wroteNamespace ||
>+    if (wroteNamespace || mExplicitUniversal ||
>         (!mIDList && !mClassList && !mPseudoClassList && !mAttrList &&
>-         (aIsNegated || !mNegations))) {
>+         aIsNegated)) {
>       aString.Append(char16_t('*'));

Why not just make the entire condition mExplicitUniversal and remove
all the other checks?

(And maybe add an assertion that !wroteNamespace || mExplicitUniversal.)

>-      if (!mNext) {
>-        // Lone pseudo-element selector -- toss in a wildcard type selector
>-        // XXXldb Why?
>+      if (mExplicitUniversal) {
>         aString.Append(char16_t('*'));
>       }

I think both the old and the new conditions are unreachable code, and
you should remove the code and add two assertions:
  NS_ASSERTION(mNext, "pseudo-elements should always have an element");
  NS_ASSERTION(!mExplicitUniversal,
               "shouldn't have other things attached to pseudo-element");
If either of those fail, I'd like to know why.



Please also test serialization for:

  p > ::first-line
  p > *::first-line
  :hover
  *:hover

as all serializing to themselves.


r=dbaron with that

	William Chen [:wchen]
	Comment 61 • 9 years ago

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #60)
> Comment on attachment 8586496 [details] [diff] [review]
> Part 3.2: Update selector serialization to preserve featureless selectors.
> Why not just make the entire condition mExplicitUniversal and remove
> all the other checks?
I kept the check for (!mIDList && !mClassList && !mPseudoClassList && !mAttrList && aIsNegated) so that :not(*) doesn't serialize to :not(). The check for wroteNamespace looks redundant so I'll remove that.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 62 • 9 years ago

Spec should be updated now per:
https://lists.w3.org/Archives/Public/www-style/2015Apr/0116.html
although I haven't reviewed the changes.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 63 • 9 years ago

Comment on attachment 8584305 [details] [diff] [review]
Part 2: Walk relevant rule in shadow roots rule processors that may have participating :host-context selectors. v2

One additional comment on this patch:  

before you set mOnlyMatchHostPseudo to true (which happens multiple places in this patch), you should assert that it's currently false.  (If there were unexpected recursion, it might not be, and your setting it back to false later would lead to bugs.)

Alternatively, maybe it would make sense to use AutoRestore.  (And we'd have to if we wanted to write C++-exception-safe code.)

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 64 • 9 years ago

Comment on attachment 8584305 [details] [diff] [review]
Part 2: Walk relevant rule in shadow roots rule processors that may have participating :host-context selectors. v2

>@@ -1205,18 +1205,19 @@ nsStyleSet::WalkRuleProcessors(nsIStyleR

>   bool cutOffInheritance = false;
>   if (mBindingManager) {
>     // We can supply additional document-level sheets that should be walked.
>     if (aWalkAllXBLStylesheets) {
>-      mBindingManager->WalkAllRules(aFunc, aData);
>+      mBindingManager->WalkAllRules(aFunc, aData, false);
>     } else {
>+      mBindingManager->WalkAllShadowRootHostRules(aFunc, aData);
>       mBindingManager->WalkRules(aFunc, aData, &cutOffInheritance);
>     }
>   }

Actually, a few more comments here:

First, I'm very suspicious of any change that touches WalkRuleProcessors without also touching FileRules.  They basically do the same thing, except FileRules is pickier about order, and creates rule nodes instead of calling an enumerator function.

Second, the code in the else also seems suspicious here, as though it's likely walking a lot more rules than needed.  In most cases, WalkRuleProcessors is used to enumerate the list of rules that might match an element (so that we can run selector matching) -- it's used when we need that list for various optimizations (for handling attribute and state changes), as opposed to FileRules, which is used when we're doing selector matching to compute style.  The aWalkAllXBLStylesheets is true for the case where we're interested in whether we have any style that depends on "document state", which is state that's not specific to any element, which in turn means we want to care about all style sheets regardless of what element they're bound to.  That's only true when called from HasDocumentStateDependentStyle.  All other callers pass false, in which case we want to return only the rules that could match the element described by aData.  The fact that WalkAllShadowHostRules works like WalkAllRules makes me pretty suspicious that it's wrong, although what these functions do could also use some better documentation.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 65 • 9 years ago

Comment on attachment 8585935 [details] [diff] [review]
Part 3.3: Only allow featureless elements to be selected by featureless element selectors.

>   // Walk the rules in shadow root for :host pseudo-class rules.
>   aData->mTreeMatchContext.mOnlyMatchHostPseudo = true;
>+  aData->mElementIsFeatureless = true;

Why is this change needed only in WalkRules and not also in WalkAllShadowHostRules?

(This might be clearer with better comments explaining what these functions were intended to do.)

Also, comment 63 applies to this boolean as well.


(I'm still working on understanding the rest of the patch.)

	William Chen [:wchen]
	Comment 66 • 9 years ago

(In reply to David Baron [:dbaron] ⏰UTC-7 from comment #64)
> Comment on attachment 8584305 [details] [diff] [review]
> Part 2: Walk relevant rule in shadow roots rule processors that may have
> participating :host-context selectors. v2
> Second, the code in the else also seems suspicious here, as though it's
> likely walking a lot more rules than needed.  In most cases,
> WalkRuleProcessors is used to enumerate the list of rules that might match
> an element (so that we can run selector matching)
:host-context(compound selector) is special in that it walks up ancestors trying to match them against the compound selector so I had to add code in a few places in order to let the restyle manager know that we may need a restyle if the ancestor changes.

Consider the following tree:
<span id="container">
<div id="host">
</div>
</span>

Where #host has a shadow root that contains the following rule:
:host-context([data-foo=bar]) { ... }

When you set attribute data-foo=bar on #container, we call RestyleManager::AttributeChanged, which will call nsStyleSet::HasAttributeDependentStyle. That method uses WalkRuleProcessors to walk rules looking for potential selector matches on the #container span. Currently we only walk shadow tree rules if the element is the host or in the shadow tree. #container is neither the host nor in the shadow tree, thus it would miss the :host-context rule that in some sense would match against #container. I added WalkAllShadowHostRules to walk shadow tree rules looking specifically for host rules. In part 4 I also added some code to AttributeEnumFunc to make the restyle hint for :host-context similar to the descendant combinator (since both walk ancestors), and in the code for :host-context matching I relax some conditions to make :host-context match against ancestors in certain cases. The reason why WalkAllShadowHostRules doesn't set mElementIsFeatureless is because the element in this case isn't necessarily the host, thus not featureless, as in the provided example.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 67 • 9 years ago

OK, I guess things would be clearer if part 2 renamed the functions.  WalkRules and WalkAllRules were previously reasonably general, but they're now accumulating more behavior that's specific to what their only caller wants, and they should probably have more specific names to match (e.g., WalkRulesForMatching, WalkRulesForDependenceChecks), which would make the code and the later patches easier to understand.  Does that make sense?

(Sorry about the delays here; in general I've been having trouble understanding these patches whenever I start looking at them.  I've been trying to come up with concrete suggestions for how to make them easier to understand, but that's not always easy to do when I don't understand what's going on in the first place.  If there are revisions that you know of that would make them easier to understand, it's probably worth doing -- although let me know that you're in the middle of doing so in comments on the bug so I don't spend time reviewing a patch you're about to replace.)

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 68 • 9 years ago

William and I talked about these patches today, and I'll take a more detailed look tomorrow.

A few conclusions:

 * I should review patches 2.1 and 3.3 merged together, since 3.3 basically stomps on much of 2.1.  (I think 2.1 also has significant bugs, e.g., with handling of things like :-moz-any(:host-context()), but at first glance those are fixed in patch 3.3.)

 * The definition of featureless has changed (see comment 62, etc.), and I need to make suggestions as to what to do about it.

 * The odd bit at the start of the :host-context() matching in part 4 can be made much more sensible by fixing a bug elsewhere in part 4.  The bug that needs fixing is that the AddSelector change at the very end of the patch needs to treat :host-context() and :-moz-any() differently.  In particular, the recursive call to AddSelector for :host-context() should pass (aCascade, s, s) instead of (aCascade, aSelectorInTopLevel, s), since :host-context is conceptually like a combinator.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 69 • 9 years ago

Comment on attachment 8585935 [details] [diff] [review]
Part 3.3: Only allow featureless elements to be selected by featureless element selectors.

In Part 3.3, it seems like you should also set mElementIsFeatureless in
the other place where you set mOnlyMatchHostPseudo, in
WalkAllShadowHostRules.  (Does the proposed change to part 4 fix the
reason you didn't do so before?)


In order to update to the new rules for featureless selectors, I think
you should drop the HasExplicitFeatures method from part 3, but
otherwise keep part 3.  And you should also drop
CanMatchFeaturelessElement from this patch.

You need a patch somewhere to allow featureless elements to match even
when they fail to match the default namespace.

I think in order to update to the new rules for featureless elements,
you should just update every relevant part (tag, ID list, class list,
pseudo class list, attr list) of SelectorMatches to return
false if there's a featureless element.  The interesting parts would
be the namespace check (see previous paragraph) where instead you'd
suppress the existing "return false" for featureless elements (and also
fix the lack of {}).  All of these could be done at the start except
for pseudo-classes, where I think you should set a boolean to true
for the ones that are ok for featureless elements, and then do the check
at the end.

Part 2.1 should also be merged into this patch.

I also think SelectorMatchesTree should explicitly fail if it's crossed
a shadow-root boundary and there's still an mNext left in the selector.
That seems like a bug in the current patch -- it seems like you would
match multiple :host-context() selectors in a row if you have a style
in a web component that's used in another web component, which is not
true per spec.  (You should probably have a test for this.)

I think the same is true for when you're matching the shadow host
directly (i.e., matching ":host-context() :host-context()" against the
shadow root, as opposed to the previous comment which is about matching
":host-context() :host-context() p" against a p inside the component).
In that case, you need SelectorMatchesTree to fail before doing any
walking.  (Probably based on mOnlyMatchHostPseudo being set already?)

Finally, it seems like something should set mOnlyMatchHostPseudo in
SelectorMatchesTree as well, no?
(Though I wonder whether we need both mOnlyMatchHostPseudo and
mElementIsFeatureless.  It seems like mOnlyMatchHostPseudo is
implementing the new definition of featureless in a somewhat odd
way and mIsFeatureless is implementing the old one.)

Otherwise I think this looks ok, but I'd like to look at the revisions.

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 70 • 9 years ago

Comment on attachment 8585935 [details] [diff] [review]
Part 3.3: Only allow featureless elements to be selected by featureless element selectors.

One more comment here:
>+  // If the node should be considered featureless (as specified in
>+  // selectors 4), then mIsFeature should be set to true to prevent
>+  // matching unless the selector is a special pseudo class or pseudo
>+  // element that matches featureless elements.
>+  const bool mIsFeatureless;

mIsFeature -> mIsFeatureless in the comment.

Though really this could convey the same information in much fewer words:
  // Whether the element is featureless (as specified in selectors 4),
  // which means that selectors do not match unless they are special
  // pseudo-classes that match featureless elements.
(I don't think there's anything about pseudo-elements, is there?)

	David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)
	Comment 71 • 9 years ago

Comment on attachment 8585930 [details] [diff] [review]
Part 4: Implement matching for :host-context selector. v2

I think the changes to ContentEnumFunc, AttributeEnumFunc, and
StateEnumFunc can go away per my comments on patch 3.3; the new
featureless checks should handle this.

AddSelector needs to be revised per comment 68 (last bullet).

In SelectorMatches, I think you can remove the whole UNKNOWN business
given the fix to AddSelector.

In SelectorMatches, the while (currentElement) { } could be instead
a do { } while (currentElement);

I think otherwise this looks good, but I'd like to look at the
revisions.

	Andrew Overholt [:overholt]
	Comment 72 • 9 years ago

William told me this is blocked on input from CSS WG people.

	gert cuykens
	Comment 73 • 8 years ago

What about `:host`? Doesn't seem to work either?

http://stackoverflow.com/questions/40601142/shadow-dom-v1-css-polyfill

	Julien Wajsberg [:julienw]
	Comment 74 • 8 years ago

":host(selector)" matches the selector on the host, while ":host-context(selector") matches the selector on the host or one of his ancestors.

See https://developers.google.com/web/fundamentals/getting-started/primers/shadowdom#contextstyling

I believe that for our specific case for Firefox OS, we could use ":host(:dir(...))". Or  even :dir if it's being inherited inside components once bug 1100912 is fixed (fortunately I didn't hold my breath).

	Emilio Cobos Álvarez (:emilio)
	Comment 75 • 7 years ago

I started to look into this. :host-context seems like a pretty broken concept to me. I opened https://github.com/w3c/csswg-drafts/issues/1914.