Bug 156397 – Implement functional :host() pseudo class

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Antti Koivisto]

We already support :host. Add functional syntax too.

[Radar WebKit Bug Importer]

<rdar://problem/25621445>

[Antti Koivisto]

Created attachment 275992 [details]
patch

[Antti Koivisto]

Created attachment 275999 [details]
patch

[WebKit Commit Bot]

Attachment 275999 [details] did not pass style-queue:


ERROR: Source/WebCore/css/ElementRuleCollector.h:92:  The parameter name "ruleRange" adds no information, so it should be removed.  [readability/parameter_name] [5]
ERROR: Source/WebCore/css/SelectorChecker.h:98:  The parameter name "checkingContext" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Antti Koivisto]

Created attachment 276000 [details]
patch

[Ryosuke Niwa]

Comment on attachment 276000 [details]
patch

The test change looks good. Maybe kling should review the style resolver change?

[Antti Koivisto]

https://trac.webkit.org/r199268

[Michael Catanzaro]

It broke debug builds without ENABLE_SHADOW_DOM:

../../Source/WebCore/css/SelectorChecker.cpp: In member function ‘bool WebCore::SelectorChecker::matchHostPseudoClass(const WebCore::CSSSelector&, const WebCore::Element&, WebCore::SelectorChecker::CheckingContext&, unsigned int&) const’:
../../Source/WebCore/css/SelectorChecker.cpp:206:94: error: ‘PseudoClassHost’ is not a member of ‘WebCore::CSSSelector’
     ASSERT(selector.match() == CSSSelector::PseudoClass && selector.pseudoClassType() == CSSSelector::PseudoClassHost);
                                                                                              ^

It's not immediately clear to me how to fix this. Disabling that assert doesn't seem right.

[Alexey Proskuryakov]

Ugh, broken build for 12 hours already. Rolling out.

[WebKit Commit Bot]

Re-opened since this is blocked by bug 156440

[Alexey Proskuryakov]

Also, surprised that Windows apparently doesn't have shadow DOM enabled.

[Ryosuke Niwa]

(In reply to comment #11)
> Also, surprised that Windows apparently doesn't have shadow DOM enabled.

Oh oops, that was my oversight. Will enable it everywhere soon.

[Antti Koivisto]

Maybe we should also enable <details> globally and remove the #ifs? That would clean up lot of conditional build mess.

[Ryosuke Niwa]

(In reply to comment #13)
> Maybe we should also enable <details> globally and remove the #ifs? That
> would clean up lot of conditional build mess.

Yeah, we've been shipping details element for a while although apparently only WebKit/Blink supports it.  This is somewhat orthogonal but I suspect no one is using it so we might be able to remove it entirely...

[Ryosuke Niwa]

(In reply to comment #14)
> (In reply to comment #13)
> > Maybe we should also enable <details> globally and remove the #ifs? That
> > would clean up lot of conditional build mess.
> 
> Yeah, we've been shipping details element for a while although apparently
> only WebKit/Blink supports it.  This is somewhat orthogonal but I suspect no
> one is using it so we might be able to remove it entirely...

Oops, apparently Gecko is implementing them now: https://bugzilla.mozilla.org/show_bug.cgi?id=1226455

[Antti Koivisto]

https://trac.webkit.org/r199291

[Michael Catanzaro]

(In reply to comment #12)
> Oh oops, that was my oversight. Will enable it everywhere soon.

Hi, do you still consider shadow DOM experimental? If so, let's enable it only in FeatureList.pm. If you think it's ready for users, only then let's additionally enable it in WebKitFeatures.cmake.

[Ryosuke Niwa]

(In reply to comment #17)
> (In reply to comment #12)
> > Oh oops, that was my oversight. Will enable it everywhere soon.
> 
> Hi, do you still consider shadow DOM experimental? If so, let's enable it
> only in FeatureList.pm. If you think it's ready for users, only then let's
> additionally enable it in WebKitFeatures.cmake.

It's getting ready to be shipped. There are still some renames to be done so I'll do that first.