Bug 156397

Summary: Implement functional :host() pseudo class
Product: WebKit Reporter: Antti Koivisto <koivisto>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, eoconnor, hyatt, kling, mcatanzaro, rniwa, simon.fraser, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 156440    
Bug Blocks:    
Attachments:
Description Flags
patch
none
patch
none
patch darin: review+

Description Antti Koivisto 2016-04-08 02:23:52 PDT
We already support :host. Add functional syntax too.
Comment 1 Radar WebKit Bug Importer 2016-04-08 02:24:29 PDT
<rdar://problem/25621445>
Comment 2 Antti Koivisto 2016-04-08 04:04:45 PDT
Created attachment 275992 [details]
patch
Comment 3 Antti Koivisto 2016-04-08 05:39:35 PDT
Created attachment 275999 [details]
patch
Comment 4 WebKit Commit Bot 2016-04-08 05:41:14 PDT
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.
Comment 5 Antti Koivisto 2016-04-08 05:42:39 PDT
Created attachment 276000 [details]
patch
Comment 6 Ryosuke Niwa 2016-04-08 17:57:31 PDT
Comment on attachment 276000 [details]
patch

The test change looks good. Maybe kling should review the style resolver change?
Comment 7 Antti Koivisto 2016-04-09 00:38:51 PDT
https://trac.webkit.org/r199268
Comment 8 Michael Catanzaro 2016-04-09 09:09:04 PDT
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.
Comment 9 Alexey Proskuryakov 2016-04-09 13:09:08 PDT
Ugh, broken build for 12 hours already. Rolling out.
Comment 10 WebKit Commit Bot 2016-04-09 13:10:14 PDT
Re-opened since this is blocked by bug 156440
Comment 11 Alexey Proskuryakov 2016-04-09 14:47:10 PDT
Also, surprised that Windows apparently doesn't have shadow DOM enabled.
Comment 12 Ryosuke Niwa 2016-04-10 21:25:14 PDT
(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.
Comment 13 Antti Koivisto 2016-04-11 00:43:12 PDT
Maybe we should also enable <details> globally and remove the #ifs? That would clean up lot of conditional build mess.
Comment 14 Ryosuke Niwa 2016-04-11 00:53:22 PDT
(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...
Comment 15 Ryosuke Niwa 2016-04-11 02:01:54 PDT
(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
Comment 16 Antti Koivisto 2016-04-11 02:13:40 PDT
https://trac.webkit.org/r199291
Comment 17 Michael Catanzaro 2016-04-11 05:45:40 PDT
(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.
Comment 18 Ryosuke Niwa 2016-04-11 09:57:08 PDT
(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.