Skip to content

Commit

Permalink
Bug 1690225 - Remove focus-visible feature flag. r=edgar
Browse files Browse the repository at this point in the history
This shipped in 85, we can remove the feature flag now. Keep
:-moz-focusring as an alias to :focus-visible at parse time.

Differential Revision: https://phabricator.services.mozilla.com/D103752
  • Loading branch information
emilio committed Feb 2, 2021
1 parent 4e494b4 commit 217ddbe
Show file tree
Hide file tree
Showing 15 changed files with 25 additions and 71 deletions.
31 changes: 5 additions & 26 deletions dom/base/nsFocusManager.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1160,6 +1160,10 @@ nsFocusManager::BlurredElementInfo::~BlurredElementInfo() = default;
static bool ShouldMatchFocusVisible(
const Element& aElement, int32_t aFocusFlags,
const Maybe<nsFocusManager::BlurredElementInfo>& aBlurredElementInfo) {
// If we were explicitly requested to show the ring, do it.
if (aFocusFlags & nsIFocusManager::FLAG_SHOWRING) {
return true;
}
// Any element which supports keyboard input (such as an input element, or any
// other element which may trigger a virtual keyboard to be shown on focus if
// a physical keyboard is not present) should always match :focus-visible when
Expand Down Expand Up @@ -1213,31 +1217,6 @@ static bool ShouldMatchFocusVisible(
return false;
}

static bool ShouldFocusRingBeVisible(
Element& aElement, int32_t aFlags,
const Maybe<nsFocusManager::BlurredElementInfo>& aBlurredElementInfo) {
if (aFlags & nsIFocusManager::FLAG_SHOWRING) {
return true;
}

const bool focusVisibleEnabled =
StaticPrefs::layout_css_focus_visible_enabled();

#if defined(XP_MACOSX) || defined(ANDROID)
if (!focusVisibleEnabled) {
// Preserve historical behavior if the focus visible pseudo-class is not
// enabled.
if (aFlags & nsIFocusManager::FLAG_BYMOUSE) {
return !nsContentUtils::ContentIsLink(&aElement) &&
!aElement.IsAnyOfHTMLElements(nsGkAtoms::video, nsGkAtoms::audio);
}
return true;
}
#endif
return focusVisibleEnabled &&
ShouldMatchFocusVisible(aElement, aFlags, aBlurredElementInfo);
}

/* static */
void nsFocusManager::NotifyFocusStateChange(
Element* aElement, Element* aElementToFocus,
Expand All @@ -1253,7 +1232,7 @@ void nsFocusManager::NotifyFocusStateChange(
if (aGettingFocus) {
EventStates eventStateToAdd = NS_EVENT_STATE_FOCUS;
if (aWindowShouldShowFocusRing ||
ShouldFocusRingBeVisible(*aElement, aFlags, aBlurredElementInfo)) {
ShouldMatchFocusVisible(*aElement, aFlags, aBlurredElementInfo)) {
eventStateToAdd |= NS_EVENT_STATE_FOCUSRING;
}
aElement->AddStates(eventStateToAdd);
Expand Down
8 changes: 3 additions & 5 deletions dom/tests/mochitest/general/test_focusrings.xhtml
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,6 @@

SimpleTest.waitForExplicitFinish();

const kFocusVisibleEnabled = SpecialPowers.getBoolPref("layout.css.focus-visible.enabled");

function snapShot(element) {
var rect = element.getBoundingClientRect();
adjustedRect = { left: rect.left - 6, top: rect.top - 6,
Expand Down Expand Up @@ -70,12 +68,12 @@ function runTest()
is(getComputedStyle($("l1"), "").outlineWidth, "0px", "appearance on previous list after focus() with :focus");

synthesizeMouse($("l1"), 4, 4, { });
checkFocus($("l1"), expectedVisible && !kFocusVisibleEnabled, "appearance on list after mouse focus with :moz-focusring");
checkFocus($("l1"), false, "appearance on list after mouse focus with :moz-focusring");
synthesizeMouse($("l2"), 4, 4, { });
checkFocus($("l2"), true, "appearance on list after mouse focus with :focus");

synthesizeMouse($("b1"), 4, 4, { });
checkFocus($("b1"), !isMac && expectedVisible && !kFocusVisibleEnabled, "appearance on button after mouse focus with :moz-focusring");
checkFocus($("b1"), false, "appearance on button after mouse focus with :moz-focusring");
if (navigator.platform.includes("Mac")) {
ok(compareSnapshots(snapShot($("b1")), snapShot($("b2")), false)[0], "focus after mouse shows no ring");
}
Expand Down Expand Up @@ -143,7 +141,7 @@ function testHTMLElements(list, isMac, expectedNoRingsOnWin)

var expectedMatchWithMouse = shouldFocus && !expectedNoRingsOnWin;
var mouseRingSize = ringSize;
if (shouldFocus && kFocusVisibleEnabled) {
if (shouldFocus) {
var textControl = (function() {
if (elem.localName == "textarea")
return true;
Expand Down
2 changes: 1 addition & 1 deletion layout/mathml/mathml.css
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ ms[rquote]:after {
text-decoration: none !important;
}

*:-moz-focusring {
:focus-visible {
/* Don't specify the outline-color, we should always use initial value. */
outline: 1px dotted;
}
Expand Down
12 changes: 6 additions & 6 deletions layout/style/res/forms.css
Original file line number Diff line number Diff line change
Expand Up @@ -492,10 +492,10 @@ input:is([type=radio], [type=checkbox]):is(:disabled, :disabled:active, :disable
cursor: unset;
}

input:not([type=file], [type=image]):-moz-focusring,
select:-moz-focusring,
button:-moz-focusring,
textarea:-moz-focusring {
input:not([type=file], [type=image]):focus-visible,
select:focus-visible,
button:focus-visible,
textarea:focus-visible {
/* These elements can handle outline themselves when themed, so we use
* outline-style: auto and skip rendering the outline only when themed and
* the theme allows so */
Expand Down Expand Up @@ -637,7 +637,7 @@ input:is([type=reset], [type=button], [type=submit]):active:hover {
border: 1px dotted transparent;
}

:-moz-focusring::-moz-focus-inner {
:focus-visible::-moz-focus-inner {
border-color: currentColor;
}

Expand Down Expand Up @@ -728,7 +728,7 @@ optgroup:before {
box-shadow: 0 0 1.5px 1px red;
}

:-moz-ui-invalid:-moz-focusring {
:-moz-ui-invalid:focus-visible {
box-shadow: 0 0 2px 2px rgba(255,0,0,0.4);
}

Expand Down
8 changes: 4 additions & 4 deletions layout/style/res/html.css
Original file line number Diff line number Diff line change
Expand Up @@ -702,14 +702,14 @@ canvas {
}

/* focusable content: anything w/ tabindex >=0 is focusable */
:-moz-focusring {
:focus-visible {
/* Don't specify the outline-color, we should always use initial value. */
outline: 1px dotted;
}

iframe:-moz-focusring,
body:-moz-focusring,
html:-moz-focusring {
iframe:focus-visible,
body:focus-visible,
html:focus-visible {
/* These elements historically don't show outlines when focused by default.
* We could consider changing that if needed. */
outline-style: none;
Expand Down
4 changes: 3 additions & 1 deletion layout/style/res/ua.css
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,10 @@
cursor: pointer;
}

*|*:any-link:-moz-focusring {
*|*:any-link:focus-visible {
/* Don't specify the outline-color, we should always use initial value. */
/* TODO(emilio): I think this is redundant, html.css does the same for all
* :focus-visible elements. */
outline: 1px dotted;
}

Expand Down
2 changes: 1 addition & 1 deletion layout/svg/svg.css
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ foreignObject {
opacity: inherit;
}

*:-moz-focusring {
:focus-visible {
/* Don't specify the outline-color, we should always use initial value. */
outline: 1px dotted;
}
Expand Down
15 changes: 1 addition & 14 deletions modules/libpref/init/StaticPrefList.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -969,9 +969,7 @@
#
# This behavior matches both historical and GTK / Windows focus behavior.
#
# :focus-visible is intended to provide better heuristics than this, so for now
# we make this false whenever layout.css.focus-visible.enabled is enabled by
# default.
# :focus-visible is intended to provide better heuristics than this.
- name: browser.display.always_show_rings_after_key_focus
type: bool
value: false
Expand Down Expand Up @@ -6082,17 +6080,6 @@
mirror: always
rust: true

# Whether the `:focus-visible` pseudo-class is enabled.
#
# NOTE: You probably want to change the default value of
# browser.display.always_show_rings_after_key_focus whenever you change the
# default value of this pref.
- name: layout.css.focus-visible.enabled
type: RelaxedAtomicBool
value: true
mirror: always
rust: true

# Whether the `:autofill` pseudo-class is exposed to content.
- name: layout.css.autofill.enabled
type: RelaxedAtomicBool
Expand Down
2 changes: 0 additions & 2 deletions servo/components/style/gecko/non_ts_pseudo_class_list.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,6 @@ macro_rules! apply_non_ts_list {
("fullscreen", Fullscreen, IN_FULLSCREEN_STATE, _),
("-moz-modal-dialog", MozModalDialog, IN_MODAL_DIALOG_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS),
("-moz-topmost-modal-dialog", MozTopmostModalDialog, IN_TOPMOST_MODAL_DIALOG_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS),
// TODO(emilio): This is inconsistently named (the capital R).
("-moz-focusring", MozFocusRing, IN_FOCUSRING_STATE, _),
("-moz-broken", MozBroken, IN_BROKEN_STATE, _),
("-moz-loading", MozLoading, IN_LOADING_STATE, _),
("-moz-has-dir-attr", MozHasDirAttr, IN_HAS_DIR_ATTR_STATE, PSEUDO_CLASS_ENABLED_IN_UA_SHEETS),
Expand Down
4 changes: 1 addition & 3 deletions servo/components/style/gecko/selector_parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,7 @@ impl NonTSPseudoClass {
"-moz-full-screen" => Some(NonTSPseudoClass::Fullscreen),
"-moz-read-only" => Some(NonTSPseudoClass::ReadOnly),
"-moz-read-write" => Some(NonTSPseudoClass::ReadWrite),
"-moz-focusring" => Some(NonTSPseudoClass::FocusVisible),
"-webkit-autofill" => Some(NonTSPseudoClass::Autofill),
_ => None,
}
Expand Down Expand Up @@ -136,9 +137,6 @@ impl NonTSPseudoClass {
/// Returns whether the pseudo-class is enabled in content sheets.
#[inline]
fn is_enabled_in_content(&self) -> bool {
if let NonTSPseudoClass::FocusVisible = *self {
return static_prefs::pref!("layout.css.focus-visible.enabled");
}
if let NonTSPseudoClass::Autofill = *self {
return static_prefs::pref!("layout.css.autofill.enabled");
}
Expand Down
1 change: 0 additions & 1 deletion servo/components/style/gecko/wrapper.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2042,7 +2042,6 @@ impl<'le> ::selectors::Element for GeckoElement<'le> {
NonTSPseudoClass::MozDragOver |
NonTSPseudoClass::MozDevtoolsHighlighted |
NonTSPseudoClass::MozStyleeditorTransitioning |
NonTSPseudoClass::MozFocusRing |
NonTSPseudoClass::MozMathIncrementScriptLevel |
NonTSPseudoClass::InRange |
NonTSPseudoClass::OutOfRange |
Expand Down
1 change: 0 additions & 1 deletion testing/web-platform/meta/css/selectors/__dir__.ini
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
prefs: [layout.css.focus-visible.enabled:true]
lsan-disabled: true
leak-threshold: [default:3276800, tab:460800]

This file was deleted.

This file was deleted.

This file was deleted.

0 comments on commit 217ddbe

Please sign in to comment.