Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use :focus-visible in "Phrasing Content" section. #6256

Merged
merged 1 commit into from
Jan 22, 2021

Conversation

emilio
Copy link
Contributor

@emilio emilio commented Dec 28, 2020

This matches Firefox, and the intent of this CSS pseudo-class, see
w3c/csswg-drafts#4278.

We should also specify that object, embed, and iframe don't have outlines on focus by default, as that's the behavior Firefox, Chromium and WebKit have, but that's a separate patch / issue.


/rendering.html ( diff )

This matches Firefox, and the intent of this CSS pseudo-class, see
w3c/csswg-drafts#4278.
@annevk annevk added topic: rendering normative change needs implementer interest Moving the issue forward requires implementers to express interest labels Jan 4, 2021
@annevk
Copy link
Member

annevk commented Jan 4, 2021

cc @whatwg/css

@domenic
Copy link
Member

domenic commented Jan 5, 2021

Interesting that this differs so much from https://chromium-review.googlesource.com/c/chromium/src/+/2602429/ . The -webkit pseudo stuff I can understand, but the html, body, embed, iframe, object, input, textarea, select styles are strange. (Not that we should block on that.)

The tests at web-platform-tests/wpt#27015 don't seem like they test the default stylesheet though. In particular there should be a test that the value of outline changes from whatever its initial value is, to auto, with no user stylesheets involved. Previously we've tested default stylesheets with getComputedStyle(); see also web-platform-tests/wpt#5625.

/cc @mrego

@emilio
Copy link
Contributor Author

emilio commented Jan 5, 2021

Interesting that this differs so much from https://chromium-review.googlesource.com/c/chromium/src/+/2602429/ . The -webkit pseudo stuff I can understand, but the html, body, embed, iframe, object, input, textarea, select styles are strange. (Not that we should block on that.)

Yeah, Blink has a lot of focus rules as well... Gecko's rules are much simpler.

The tests at web-platform-tests/wpt#27015 don't seem like they test the default stylesheet though. In particular there should be a test that the value of outline changes from whatever its initial value is, to auto, with no user stylesheets involved. Previously we've tested default stylesheets with getComputedStyle(); see also web-platform-tests/wpt#5625.

Well, they do test the computed styles (or lack thereof on this case). In particular, they test that a focused element via mouse (which should match focus but not focus-visible) has no outline, which is the desired behavior and a behavior difference from this patch.

@domenic
Copy link
Member

domenic commented Jan 6, 2021

Well, they do test the computed styles (or lack thereof on this case). In particular, they test that a focused element via mouse (which should match focus but not focus-visible) has no outline, which is the desired behavior and a behavior difference from this patch.

It's the "lack thereof" that I'm concerned about. I'd like to have a test which distinguishes between outline: auto and outline: none or outline: thick double #32a1ce or outline: 1px dotted. I believe a browser which used any of those others in their :focus-visible UA stylesheet would pass the test in question, which is bad.

mrego added a commit to mrego/wpt that referenced this pull request Jan 8, 2021
This test checks that when you focus an element via script,
it show a focus ring with `outline-style: auto`.
See: w3c/csswg-drafts#4278 & whatwg/html#6256

Currently Chromium and WebKit pass this test,
because despite they don't use `:focus-visible` in the UA stylesheet,
they're painting an auto style outline when an element is focused.
However Firefox fails it, because even when it uses `:-moz-focusring`
(the equivalent to `:focus-visible`) in the UA stylesheet,
it uses dotted style for the outline.
mrego added a commit to mrego/wpt that referenced this pull request Jan 8, 2021
This test checks that when you focus an element via script,
it show a focus ring with `outline-style: auto`.
See: w3c/csswg-drafts#4278 & whatwg/html#6256

Currently Chromium passes this test,
because despite they don't use `:focus-visible` in the UA stylesheet,
it's painting an auto style outline when an element is focused.
However Firefox fails it, because even when it uses `:-moz-focusring`
(the equivalent to `:focus-visible`) in the UA stylesheet,
it uses dotted style for the outline.
WebKit doesn't support `:focus-visible` yet an it fails,
thought it's painting an auto style outline
(the background color doesn't match on the test).
mrego added a commit to mrego/wpt that referenced this pull request Jan 8, 2021
This test checks that when you focus an element via script,
it show a focus ring with `outline-style: auto`.
See: w3c/csswg-drafts#4278 & whatwg/html#6256

Currently Chromium passes this test,
because despite they don't use `:focus-visible` in the UA stylesheet,
it's painting an auto style outline when an element is focused.
However Firefox fails it, because even when it uses `:-moz-focusring`
(the equivalent to `:focus-visible`) in the UA stylesheet,
it uses dotted style for the outline.
WebKit doesn't support `:focus-visible` yet an it fails,
thought it's painting an auto style outline
(the background color doesn't match on the test).
@mrego
Copy link
Member

mrego commented Jan 8, 2021

Regarding the Chromium default stylesheet and the complicated rules there, I guess that's something out of the scope of this PR. Probably a Chromium bug is the proper place to discuss that.

Regarding the tests, the initial test in web-platform-tests/wpt#27015 (focus-visisble-013.html) is passing in Firefox and failing in Chromium. That's expected as Chromium is using :focus in the UA stylesheet.
I've added a new test in the PR that checks that the outline-style is auto (focus-visisble-017.html), but that test right now passes in Chromium (though Chromium uses :focus pseudo-class) and fails in Firefox, because it uses outline: 1px dotted; so the style is dotted. I guess that kind of test is what @domenic was asking for.

@domenic
Copy link
Member

domenic commented Jan 8, 2021

Regarding the Chromium default stylesheet and the complicated rules there, I guess that's something out of the scope of this PR. Probably a Chromium bug is the proper place to discuss that.

Well, if we think the spec should be changed, we'll want a spec bug. (But I'm not sure whether we do.) I agree it's out of scope for this PR.

I've added a new test in the PR that checks that the outline-style is auto (focus-visisble-017.html), but that test right now passes in Chromium (though Chromium uses :focus pseudo-class) and fails in Firefox, because it uses outline: 1px dotted; so the style is dotted. I guess that kind of test is what @domenic was asking for.

Perfect. Could you link to the test pull request?

@mrego
Copy link
Member

mrego commented Jan 8, 2021

Perfect. Could you link to the test pull request?

I've put both test files on the same PR: web-platform-tests/wpt#27015.

Copy link
Member

@domenic domenic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. I guess we should wait for an answer to https://bugs.chromium.org/p/chromium/issues/detail?id=1162070#c7 before merging, to make sure there isn't some objection from other Chromium style folks. (But, I'm pretty sure there won't be.)

mrego added a commit to mrego/wpt that referenced this pull request Jan 12, 2021
This test checks that when you focus an element via script,
it show a focus ring with `outline-style: auto`.
See: w3c/csswg-drafts#4278 & whatwg/html#6256

Currently Chromium passes this test,
because despite they don't use `:focus-visible` in the UA stylesheet,
it's painting an auto style outline when an element is focused.
However Firefox fails it, because even when it uses `:-moz-focusring`
(the equivalent to `:focus-visible`) in the UA stylesheet,
it uses dotted style for the outline.
WebKit doesn't support `:focus-visible` yet an it fails,
thought it's painting an auto style outline
(the background color doesn't match on the test).
Base automatically changed from master to main January 15, 2021 07:58
@mrego
Copy link
Member

mrego commented Jan 22, 2021

So @lilles has replied on the Chromium bug, and @alice has also shown agreement on the tests and CSSWG issue.

@domenic is that enough to unblock this? or do we need something else? Thanks.

mrego added a commit to mrego/wpt that referenced this pull request Jan 22, 2021
This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).
mrego added a commit to mrego/wpt that referenced this pull request Jan 22, 2021
This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).
@domenic
Copy link
Member

domenic commented Jan 22, 2021

Yes, thank you for the ping! Let's get this merged.

@domenic domenic merged commit f6bc62f into whatwg:main Jan 22, 2021
@domenic domenic added topic: focus and removed needs implementer interest Moving the issue forward requires implementers to express interest labels Jan 22, 2021
mrego added a commit to web-platform-tests/wpt that referenced this pull request Jan 25, 2021
This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this pull request Feb 2, 2021
… in the default UA style sheet, a=testonly

Automatic update from web-platform-tests
[selectors] Add tests for :focus-visible in the default UA style sheet

This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).

--

wpt-commits: 08069be5028d00518abd36e132275252937a34d3
wpt-pr: 27015
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified-and-comments-removed that referenced this pull request Feb 3, 2021
… in the default UA style sheet, a=testonly

Automatic update from web-platform-tests
[selectors] Add tests for :focus-visible in the default UA style sheet

This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).

--

wpt-commits: 08069be5028d00518abd36e132275252937a34d3
wpt-pr: 27015

UltraBlame original commit: 990ee1b12d5a1243273aa37a90bd99326eb8dcf8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-comments-removed that referenced this pull request Feb 3, 2021
… in the default UA style sheet, a=testonly

Automatic update from web-platform-tests
[selectors] Add tests for :focus-visible in the default UA style sheet

This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).

--

wpt-commits: 08069be5028d00518abd36e132275252937a34d3
wpt-pr: 27015

UltraBlame original commit: 990ee1b12d5a1243273aa37a90bd99326eb8dcf8
gecko-dev-updater pushed a commit to marco-c/gecko-dev-wordified that referenced this pull request Feb 3, 2021
… in the default UA style sheet, a=testonly

Automatic update from web-platform-tests
[selectors] Add tests for :focus-visible in the default UA style sheet

This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).

--

wpt-commits: 08069be5028d00518abd36e132275252937a34d3
wpt-pr: 27015

UltraBlame original commit: 990ee1b12d5a1243273aa37a90bd99326eb8dcf8
jamienicol pushed a commit to jamienicol/gecko that referenced this pull request Feb 4, 2021
… in the default UA style sheet, a=testonly

Automatic update from web-platform-tests
[selectors] Add tests for :focus-visible in the default UA style sheet

This patch adds 2 new tests to verify that the default UA style sheet
uses `:focus-visible { outline: auto; }`.
See: whatwg/html#6256 & w3c/csswg-drafts#4278

* focus-visible-017.html:
  This test checks that when you focus an element via script,
  it show a focus ring with `outline-style: auto`.
  Currently Chromium passes this test,
  because despite they don't use `:focus-visible` in the UA stylesheet,
  it's painting an auto style outline when an element is focused.
  However Firefox fails it, because even when it uses `:-moz-focusring`
  (the equivalent to `:focus-visible`) in the UA stylesheet,
  it uses dotted style for the outline.
  WebKit doesn't support `:focus-visible` yet an it fails,
  thought it's painting an auto style outline
  (the test is specifically checking for `:focus-visible` support).

* focus-visible-018.html:
  This test checks that when you click an element to focus it,
  it doesn't show any kind of focus ring.
  Currently Firefox passes this test, by Chromium fails it
  because Chromium is using `:focus` on the default UA stylesheet
  and is adding an outline on the element, despite it doesn't match
  `:focus-visible` (see https://crbug.com/1162070).

--

wpt-commits: 08069be5028d00518abd36e132275252937a34d3
wpt-pr: 27015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

4 participants