[selectors4] Implement :focus-within pseudo-class

[selectors4] Implement :focus-within pseudo-class

This patch adds support for the new ":focus-within" pseudo-class.
Most of the patch is basically the regular boilerplate code
to add a new selector. Then the interesting changes happen
on ContainerNode.

The patch is covered by a bunch of tests imported from
the W3C test suite (which includes generic tests in addition to
the tests from Firefox and WebKit implementations too).
Apart from some minor changes on current tests to add
the new selector.

Intent-to-ship thread is available at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg

BUG=617371
Review-Url: https://codereview.chromium.org/2795143004
Cr-Commit-Position: refs/heads/master@{#463992}
Committed: https://chromium.googlesource.com/chromium/src/+/ffc655c99fef91f49ad7906344d51678c3853b74

[Manuel Rego, 2017-04-04 09:55:15]

rego@igalia.com changed reviewers:
+ kojii@chromium.org, meade@chromium.org, rune@opera.com

[Manuel Rego, 2017-04-04 09:55:15]

This is the first version of the patch to add support for ":focus-within".
As mentioned in the description this passes all the tests from W3C
which includes some generic tests, Firefox tests and WebKit ones
(some still pending to be reviewed).

Some stuff before we can land this:
* There are a few PRs adding more tests for this feature
  pending to be reviewed at WPT repo:
 
https://github.com/w3c/web-platform-tests/pulls?q=is%3Apr+is%3Aopen+label%3As...
* The WPT tests are being imported as manual tests
  (when they could be run automatically).
  We'd need to solve the following issue or generate rebaselines for them
  at this point: https://github.com/w3c/wpt-tools/issues/196
* We still need 1 more LGTM on the intent-to-ship thread.
  It was somehow pending on a spec issue that hopefully
  will be quickly solved: https://github.com/w3c/csswg-drafts/issues/1135

Despite all this I think the patch is ready to be reviewed
and I'd love to get your feedback.
Meanwhile I'll try to move forward the other things in parallel.

[rune, 2017-04-04 13:00:27]

Just a preliminary question: did you consider if avoiding style invalidation of
a common ancestor chain when changing focus, like we do for hover, would be a
perf problem?

[meade_UTC10, 2017-04-05 06:59:58]

This all looks pretty straight forward and fine to me, but it'd be great to get
further opinions from some of the more experienced folks (eg kojii and rune)

Thanks for working on this, rego!

[Manuel Rego, 2017-04-05 07:43:09]

On 2017/04/04 13:00:27, rune wrote:
> Just a preliminary question: did you consider if avoiding style invalidation
of
> a common ancestor chain when changing focus, like we do for hover, would be a
> perf problem?

Thanks @rune for the suggestion.

I didn't investigate this so far. I'd give it a try maybe
we can do something similar to what's done in :hover without too much trouble.
Anyway I believe changing focus is not as frequent as changing
the element being hovered (just moving the mouse around the page),
so maybe the performance impact is not so important.
I guess a huge DOM with a <form> very deep on the tree will benefit from that.

[rune, 2017-04-05 09:09:58]

On 2017/04/05 07:43:09, Manuel Rego wrote:
> On 2017/04/04 13:00:27, rune wrote:
> > Just a preliminary question: did you consider if avoiding style invalidation
> of
> > a common ancestor chain when changing focus, like we do for hover, would be
a
> > perf problem?
> 
> Thanks @rune for the suggestion.
> 
> I didn't investigate this so far. I'd give it a try maybe
> we can do something similar to what's done in :hover without too much trouble.
> Anyway I believe changing focus is not as frequent as changing
> the element being hovered (just moving the mouse around the page),
> so maybe the performance impact is not so important.

Possibly not, depends on how the selector is being used.

> I guess a huge DOM with a <form> very deep on the tree will benefit from that.

The case could typically be focusing between siblings input elements in a form
where you have a :focus-within rule on body for instance. Like having an
positioned tooltip on body indicating that you are filling out a form. Changing
focus between input elements in a form would only consider style invalidation
for elements inside that form under the common ancestor for the input elements.


I can't see that the layout tree detach for the focused element is being handled
like we do for :active and :hover?

I think it would be a good idea to have this feature behind a runtime flag so
that we can land patches without being worried that we ship an implementation
which is not complete. I think there can be some trickiness to this selector for
dynamic changes.

[meade_UTC10, 2017-04-06 03:38:08]

(Moving myself to cc since it looks like Rune is looking at this one - thanks
Rune!)

[meade_UTC10, 2017-04-06 03:38:33]

meade@chromium.org changed reviewers:
- meade@chromium.org

[Manuel Rego, 2017-04-07 08:17:44]

Just an update on how things from first comment are moving,
and some quick thoughts on @rune's feedback.

On 2017/04/04 09:55:15, Manuel Rego wrote:
> Some stuff before we can land this:
> * There are a few PRs adding more tests for this feature
>   pending to be reviewed at WPT repo:
>  
>
https://github.com/w3c/web-platform-tests/pulls?q=is%3Apr+is%3Aopen+label%3As...

The tests has been reviewed and approved upstream.
All of them are merged but one, which is pending the spec change
(flat tree vs shadow including descendants).

> * The WPT tests are being imported as manual tests
>   (when they could be run automatically).
>   We'd need to solve the following issue or generate rebaselines for them
>   at this point: https://github.com/w3c/wpt-tools/issues/196

It seems this will be solved, a PR is ready and approved,
but some issues with tools in WPT repo are delaying its merge.

> * We still need 1 more LGTM on the intent-to-ship thread.
>   It was somehow pending on a spec issue that hopefully
>   will be quickly solved: https://github.com/w3c/csswg-drafts/issues/1135

We already have 3 LGTMs there. The spec issue is not resolved yet
and blocking one test as mentioned before.

My plan is to re-enable the selectors4/ tests from WPT 
and then upload a new version of this patch for review.

On 2017/04/05 09:09:58, rune wrote:
> The case could typically be focusing between siblings input elements in a form
> where you have a :focus-within rule on body for instance. Like having an
> positioned tooltip on body indicating that you are filling out a form.
Changing
> focus between input elements in a form would only consider style invalidation
> for elements inside that form under the common ancestor for the input
elements.

Yeah it seems clear the common ancestor optimization would be good in this case.
I've give it an initial try, modifying Document::setFocusedElement()
in a similar way to what's done for :hover.
However ContainerNode::setFocused() is called from different places, e.g.
FocusController::dispatchEventsOnWindowAndFocusedElement(),
so I'll need to check it more thoroughly.

> I can't see that the layout tree detach for the focused element is being
handled
> like we do for :active and :hover?

I'm not sure if we need to do the same for this case.
We've now a test that passes with current patch that sets "display: none"
on the focused element and also detach them from the tree:
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...

Dunno if you're thinking in other cases.

The trick regarding setting "display: none" is that the element
doesn't lose the focus. And that's what happens in all browsers:
http://jsbin.com/hetuvig/edit?html,output

Check the following IRC log for more info:
http://logs.glob.uno/?c=freenode%23whatwg&s=5+Apr+2017#c1024801

> I think it would be a good idea to have this feature behind a runtime flag so
> that we can land patches without being worried that we ship an implementation
> which is not complete. I think there can be some trickiness to this selector
for
> dynamic changes.

Ok, I've added the runtime flag, so we can land patches incrementally.

[rune, 2017-04-07 09:45:20]

On 2017/04/07 08:17:44, Manuel Rego wrote:

> > I can't see that the layout tree detach for the focused element is being
> handled
> > like we do for :active and :hover?
> 
> I'm not sure if we need to do the same for this case.
> We've now a test that passes with current patch that sets "display: none"
> on the focused element and also detach them from the tree:
>
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...
> 
> Dunno if you're thinking in other cases.

No, I thought you'd lose focus going to display:none. Blink does.

> The trick regarding setting "display: none" is that the element
> doesn't lose the focus. And that's what happens in all browsers:
> http://jsbin.com/hetuvig/edit?html,output
> 
> Check the following IRC log for more info:
> http://logs.glob.uno/?c=freenode%23whatwg&s=5+Apr+2017#c1024801

Blink does lose focus (but it happens in a timer, so you won't see it
sychronously):

<!DOCTYPE html>
<style>
    input.none { display: none }
    input:focus + span { color: green }
</style>
<input type="text"></input><span>sibling is focused if this text is green</span>
<script>
    document.querySelector("input").focus()
    setTimeout(() => document.querySelector("input").className = "none", 1000);
    setTimeout(() => console.log(document.activeElement), 2000);
</script>

[frivoal, 2017-04-07 09:53:27]

On 2017/04/07 09:45:20, rune wrote:
> On 2017/04/07 08:17:44, Manuel Rego wrote:
> 
> > > I can't see that the layout tree detach for the focused element is being
> > handled
> > > like we do for :active and :hover?
> > 
> > I'm not sure if we need to do the same for this case.
> > We've now a test that passes with current patch that sets "display: none"
> > on the focused element and also detach them from the tree:
> >
>
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...
> > 
> > Dunno if you're thinking in other cases.
> 
> No, I thought you'd lose focus going to display:none. Blink does.
> 
> > The trick regarding setting "display: none" is that the element
> > doesn't lose the focus. And that's what happens in all browsers:
> > http://jsbin.com/hetuvig/edit?html,output
> > 
> > Check the following IRC log for more info:
> > http://logs.glob.uno/?c=freenode%23whatwg&s=5+Apr+2017#c1024801
> 
> Blink does lose focus (but it happens in a timer, so you won't see it
> sychronously):

According to AnneVK (see IRC log above), in the absence of anything in the spec
saying that focus should be lost when thing become display none, then they are
not supposed to and this is a bug.

If you're not willing to consider this a bug, I suppose we could change the TC
to check that IF the focus isn't lost by going display none, then :focus-within
still applies as well.

Or we just ignore this, because the test is synchronous? Sounds like the path of
least resistance, but not the ideal one.

[rune, 2017-04-07 10:37:45]

On 2017/04/07 09:53:27, frivoal wrote:
> On 2017/04/07 09:45:20, rune wrote:

> > Blink does lose focus (but it happens in a timer, so you won't see it
> > sychronously):
> 
> According to AnneVK (see IRC log above), in the absence of anything in the
spec
> saying that focus should be lost when thing become display none, then they are
> not supposed to and this is a bug.

Yes, I just wanted to point out the interop analysis was wrong.

> If you're not willing to consider this a bug, I suppose we could change the TC
> to check that IF the focus isn't lost by going display none, then
:focus-within
> still applies as well.
> 
> Or we just ignore this, because the test is synchronous? Sounds like the path
of
> least resistance, but not the ideal one.

Fixing this bug seems like just removing code/complexity unless there's some
content relying on this, so I think we should try. Leave the test as it is.

[Manuel Rego, 2017-04-07 10:39:08]

On 2017/04/07 09:53:27, frivoal wrote:
> On 2017/04/07 09:45:20, rune wrote:
> > On 2017/04/07 08:17:44, Manuel Rego wrote:
> > 
> > > > I can't see that the layout tree detach for the focused element is being
> > > handled
> > > > like we do for :active and :hover?
> > > 
> > > I'm not sure if we need to do the same for this case.
> > > We've now a test that passes with current patch that sets "display: none"
> > > on the focused element and also detach them from the tree:
> > >
> >
>
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...
> > > 
> > > Dunno if you're thinking in other cases.
> > 
> > No, I thought you'd lose focus going to display:none. Blink does.
> > 
> > > The trick regarding setting "display: none" is that the element
> > > doesn't lose the focus. And that's what happens in all browsers:
> > > http://jsbin.com/hetuvig/edit?html,output
> > > 
> > > Check the following IRC log for more info:
> > > http://logs.glob.uno/?c=freenode%23whatwg&s=5+Apr+2017#c1024801
> > 
> > Blink does lose focus (but it happens in a timer, so you won't see it
> > sychronously):
> 
> According to AnneVK (see IRC log above), in the absence of anything in the
spec
> saying that focus should be lost when thing become display none, then they are
> not supposed to and this is a bug.
> 
> If you're not willing to consider this a bug, I suppose we could change the TC
> to check that IF the focus isn't lost by going display none, then
:focus-within
> still applies as well.
> 
> Or we just ignore this, because the test is synchronous? Sounds like the path
of
> least resistance, but not the ideal one.

Wow, this is something I need to investigate. Specially if the only browser
doing this is Chrome...

[rune, 2017-04-07 12:01:36]

On 2017/04/07 10:39:08, Manuel Rego wrote:
> On 2017/04/07 09:53:27, frivoal wrote:
> > On 2017/04/07 09:45:20, rune wrote:
> > > On 2017/04/07 08:17:44, Manuel Rego wrote:
> > > 
> > > > > I can't see that the layout tree detach for the focused element is
being
> > > > handled
> > > > > like we do for :active and :hover?
> > > > 
> > > > I'm not sure if we need to do the same for this case.
> > > > We've now a test that passes with current patch that sets "display:
none"
> > > > on the focused element and also detach them from the tree:
> > > >
> > >
> >
>
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...
> > > > 
> > > > Dunno if you're thinking in other cases.
> > > 
> > > No, I thought you'd lose focus going to display:none. Blink does.
> > > 
> > > > The trick regarding setting "display: none" is that the element
> > > > doesn't lose the focus. And that's what happens in all browsers:
> > > > http://jsbin.com/hetuvig/edit?html,output
> > > > 
> > > > Check the following IRC log for more info:
> > > > http://logs.glob.uno/?c=freenode%23whatwg&s=5+Apr+2017#c1024801
> > > 
> > > Blink does lose focus (but it happens in a timer, so you won't see it
> > > sychronously):
> > 
> > According to AnneVK (see IRC log above), in the absence of anything in the
> spec
> > saying that focus should be lost when thing become display none, then they
are
> > not supposed to and this is a bug.
> > 
> > If you're not willing to consider this a bug, I suppose we could change the
TC
> > to check that IF the focus isn't lost by going display none, then
> :focus-within
> > still applies as well.
> > 
> > Or we just ignore this, because the test is synchronous? Sounds like the
path
> of
> > least resistance, but not the ideal one.
> 
> Wow, this is something I need to investigate. Specially if the only browser
> doing this is Chrome...

I checked WebKit which doesn't do this. However, I found
https://crbug.com/491828 which refers to
https://html.spec.whatwg.org/multipage/interaction.html#focus-fixup-rule-one

[Manuel Rego, 2017-04-10 11:55:29]

On 2017/04/07 12:01:36, rune wrote:
> I checked WebKit which doesn't do this. However, I found
> https://crbug.com/491828 which refers to
> https://html.spec.whatwg.org/multipage/interaction.html#focus-fixup-rule-one

Thanks for the pointer @rune.

So it seems Chrome is the only browser behaving like that,
but is the only one following the spec.
There are bugs reported in the rest of browsers as you can see at:
https://bugs.chromium.org/p/chromium/issues/detail?id=491828#c4

This means that probably we should update the test to match this behavior.

Apart from that, I've rebased the patch and uploaded it again adding
one test to verify that :focus-within is also lost after "display: none".

Regarding the rest of the tests, they've been reviewed and merged on WPT
and are about to be imported, then I'll rebase the patch again and remove them
from TestExpectations.

[Manuel Rego, 2017-04-11 08:57:40]

Ok, so the tests have been imported and are passing
(1 is pending but it'll be coming in the next import
as it has already landed upstream).

I added specific tests for "display: none" to verify that :focus-within
follows :focus there; and the inactive window, as I was having issues
with that when trying the common ancestor approach.

Regarding the test imported from WebKit, my plan is to update it upstream
to add a timeout and check that the element is unfocused after "display: none":
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...

@rune do you think this is ready to review, or should we include more stuff
in the initial patch?

Thanks!

[rune, 2017-04-11 09:05:27]

https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
(right):

https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:19:
test(() => assert_flase(input.matches(":focus-within")),
assert_flase?

https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:21:
}, 0);
Are you sure this won't be flaky?

This is because focus is cleared based on a 0-delayed timer triggered by
ClearFocusedElementSoon()?

[rune, 2017-04-11 09:15:10]

https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
File
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
(right):

https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:21:
}, 0);
On 2017/04/11 09:05:27, rune wrote:
> Are you sure this won't be flaky?
> 
> This is because focus is cleared based on a 0-delayed timer triggered by
> ClearFocusedElementSoon()?

Perhaps using rAF would be more correct?

Also, shouldn't these tests be async?

[Manuel Rego, 2017-04-11 09:28:59]

On 2017/04/11 09:15:10, rune wrote:
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
> File
> third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
> (right):
> 
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
>
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:21:
> }, 0);
> On 2017/04/11 09:05:27, rune wrote:
> > Are you sure this won't be flaky?
> > 
> > This is because focus is cleared based on a 0-delayed timer triggered by
> > ClearFocusedElementSoon()?
> 
> Perhaps using rAF would be more correct?

Do you mean using requestAnimationFrame()?
I can give it a try.

> Also, shouldn't these tests be async?

Yeah I broke this test so badly. Originally I was not using testharness,
but later when I moved to testharness I did it really wrong.
Indeed it has to be an asycn_test, that was why I was not getting a syntax error
with assert_flase(). I'll upload a new version fixing this.

Regarding flakiness I run it a lot of iterations locally and it passed always,
but yeah it can be flacky in some platform I guess.

[rune, 2017-04-11 10:14:26]

On 2017/04/11 09:28:59, Manuel Rego wrote:
> On 2017/04/11 09:15:10, rune wrote:
> >
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
> > File
> > third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
> > (right):
> > 
> >
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
> >
>
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:21:
> > }, 0);
> > On 2017/04/11 09:05:27, rune wrote:
> > > Are you sure this won't be flaky?
> > > 
> > > This is because focus is cleared based on a 0-delayed timer triggered by
> > > ClearFocusedElementSoon()?
> > 
> > Perhaps using rAF would be more correct?
> 
> Do you mean using requestAnimationFrame()?
> I can give it a try.

Yes. I wonder if you perhaps need two (nested) rAFs if the timeout is triggered
when generating the first frame.

> > Also, shouldn't these tests be async?
> 
> Yeah I broke this test so badly. Originally I was not using testharness,
> but later when I moved to testharness I did it really wrong.
> Indeed it has to be an asycn_test, that was why I was not getting a syntax
error
> with assert_flase(). I'll upload a new version fixing this.
> 
> Regarding flakiness I run it a lot of iterations locally and it passed always,
> but yeah it can be flacky in some platform I guess.

[Manuel Rego, 2017-04-11 10:24:11]

On 2017/04/11 10:14:26, rune wrote:
> On 2017/04/11 09:28:59, Manuel Rego wrote:
> > On 2017/04/11 09:15:10, rune wrote:
> > >
> >
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
> > > File
> > >
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html
> > > (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/2795143004/diff/100001/third_party/WebKit/Lay...
> > >
> >
>
third_party/WebKit/LayoutTests/fast/selectors/focus-within-display-none.html:21:
> > > }, 0);
> > > On 2017/04/11 09:05:27, rune wrote:
> > > > Are you sure this won't be flaky?
> > > > 
> > > > This is because focus is cleared based on a 0-delayed timer triggered by
> > > > ClearFocusedElementSoon()?
> > > 
> > > Perhaps using rAF would be more correct?
> > 
> > Do you mean using requestAnimationFrame()?
> > I can give it a try.
> 
> Yes. I wonder if you perhaps need two (nested) rAFs if the timeout is
triggered
> when generating the first frame.

Yeah you're right. I've just uploaded a new version of the test.
This time I'm sure it's testing something at least (I've checked it
manually with "t.step(() => assert_true(false));" just before "t.done()"). :-)

[rune, 2017-04-11 10:38:02]

lgtm if bots are green, but please fix the typos in the description:

[selctors4] -> [selectors4]
bolierplate -> boilerplate

[Manuel Rego, 2017-04-11 10:43:49]

Description was changed from

==========
[selctors4] Implement :focus-within pseudo-class

This patch adds support for the new ":focus-within" pseudo-class.
Most of the patch is basically the regular bolierplate code
to add a new selector. Then the interesting changes happen
on ContainerNode.

The patch is covered by a bunch of tests imported from
the W3C test suite (which includes generic tests in addition to
the tests from Firefox and WebKit implementations too).
Apart from some minor changes on current tests to add
the new selector.

Intent-to-ship thread is available at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg

BUG=617371
==========

to

==========
[selectors4] Implement :focus-within pseudo-class

This patch adds support for the new ":focus-within" pseudo-class.
Most of the patch is basically the regular boilerplate code
to add a new selector. Then the interesting changes happen
on ContainerNode.

The patch is covered by a bunch of tests imported from
the W3C test suite (which includes generic tests in addition to
the tests from Firefox and WebKit implementations too).
Apart from some minor changes on current tests to add
the new selector.

Intent-to-ship thread is available at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg

BUG=617371
==========

[Manuel Rego, 2017-04-11 12:49:14]

On 2017/04/11 10:38:02, rune wrote:
> lgtm if bots are green, but please fix the typos in the description:

Thanks @rune for the thoroughly review.

> [selctors4] -> [selectors4]
> bolierplate -> boilerplate

Fixed.

The next steps in my plan would be:
* Fix the test on WPT:
 
https://github.com/w3c/web-platform-tests/blob/master/css/selectors4/focus-wi...
* Investigate the common ancestor approach deeply.

[Manuel Rego, 2017-04-11 12:49:21]

The CQ bit was checked by rego@igalia.com

[commit-bot: I haz the power, 2017-04-11 12:49:39]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-11 12:57:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-11 12:57:54]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Manuel Rego, 2017-04-11 13:09:25]

rego@igalia.com changed reviewers:
+ eae@chromium.org, pdr@chromium.org, tkent@chromium.org

[Manuel Rego, 2017-04-11 13:09:26]

Opps, I miss an OWNER for the change on RuntimeEnabledFeatures.json5.

Adding a few people on CC, please take a look. Thanks!

[eae, 2017-04-12 09:05:06]

LGTM

[eae, 2017-04-12 09:05:13]

The CQ bit was unchecked by eae@chromium.org

[eae, 2017-04-12 09:05:14]

The CQ bit was checked by eae@chromium.org

[eae, 2017-04-12 09:05:14]

The CQ bit was checked by eae@chromium.org

[commit-bot: I haz the power, 2017-04-12 09:05:38]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 09:13:31]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2017-04-12 09:13:32]

Try jobs failed on following builders:
  chromium_presubmit on master.tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/chromium_presub...)

[Manuel Rego, 2017-04-12 09:33:40]

The CQ bit was checked by rego@igalia.com

[Manuel Rego, 2017-04-12 09:33:40]

The patchset sent to the CQ was uploaded after l-g-t-m from eae@chromium.org,
rune@opera.com
Link to the patchset: https://codereview.chromium.org/2795143004/#ps140001
(title: "Rebased patch")

[commit-bot: I haz the power, 2017-04-12 09:34:14]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-12 11:31:34]

CQ is committing da patch.

Bot data: {"patchset_id": 140001, "attempt_start_ts": 1491989620196280,
"parent_rev": "57cee813cd7d1c400275a5cb8e95612a4fae0fd4", "commit_rev":
"ffc655c99fef91f49ad7906344d51678c3853b74"}

[commit-bot: I haz the power, 2017-04-12 11:34:13]

Description was changed from

==========
[selectors4] Implement :focus-within pseudo-class

This patch adds support for the new ":focus-within" pseudo-class.
Most of the patch is basically the regular boilerplate code
to add a new selector. Then the interesting changes happen
on ContainerNode.

The patch is covered by a bunch of tests imported from
the W3C test suite (which includes generic tests in addition to
the tests from Firefox and WebKit implementations too).
Apart from some minor changes on current tests to add
the new selector.

Intent-to-ship thread is available at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg

BUG=617371
==========

to

==========
[selectors4] Implement :focus-within pseudo-class

This patch adds support for the new ":focus-within" pseudo-class.
Most of the patch is basically the regular boilerplate code
to add a new selector. Then the interesting changes happen
on ContainerNode.

The patch is covered by a bunch of tests imported from
the W3C test suite (which includes generic tests in addition to
the tests from Firefox and WebKit implementations too).
Apart from some minor changes on current tests to add
the new selector.

Intent-to-ship thread is available at:
https://groups.google.com/a/chromium.org/forum/#!topic/blink-dev/V3RNBhQelSg

BUG=617371

Review-Url: https://codereview.chromium.org/2795143004
Cr-Commit-Position: refs/heads/master@{#463992}
Committed:
https://chromium.googlesource.com/chromium/src/+/ffc655c99fef91f49ad7906344d5...
==========

[commit-bot: I haz the power, 2017-04-12 11:34:14]

Committed patchset #8 (id:140001) as
https://chromium.googlesource.com/chromium/src/+/ffc655c99fef91f49ad7906344d5...