From 217ddbe75f1d40c513eeb654c09b12c0c9511acf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Emilio=20Cobos=20=C3=81lvarez?= Date: Tue, 2 Feb 2021 23:45:25 +0000 Subject: [PATCH] Bug 1690225 - Remove focus-visible feature flag. r=edgar 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 --- dom/base/nsFocusManager.cpp | 31 +++---------------- .../mochitest/general/test_focusrings.xhtml | 8 ++--- layout/mathml/mathml.css | 2 +- layout/style/res/forms.css | 12 +++---- layout/style/res/html.css | 8 ++--- layout/style/res/ua.css | 4 ++- layout/svg/svg.css | 2 +- modules/libpref/init/StaticPrefList.yaml | 15 +-------- .../style/gecko/non_ts_pseudo_class_list.rs | 2 -- .../components/style/gecko/selector_parser.rs | 4 +-- servo/components/style/gecko/wrapper.rs | 1 - .../meta/css/selectors/__dir__.ini | 1 - .../drawFocusIfNeeded_001.html.ini | 2 -- .../drawFocusIfNeeded_004.html.ini | 2 -- .../drawFocusIfNeeded_005.html.ini | 2 -- 15 files changed, 25 insertions(+), 71 deletions(-) delete mode 100644 testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_001.html.ini delete mode 100644 testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_004.html.ini delete mode 100644 testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_005.html.ini diff --git a/dom/base/nsFocusManager.cpp b/dom/base/nsFocusManager.cpp index ca474edb3ee0..425e38ae0d22 100644 --- a/dom/base/nsFocusManager.cpp +++ b/dom/base/nsFocusManager.cpp @@ -1160,6 +1160,10 @@ nsFocusManager::BlurredElementInfo::~BlurredElementInfo() = default; static bool ShouldMatchFocusVisible( const Element& aElement, int32_t aFocusFlags, const Maybe& 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 @@ -1213,31 +1217,6 @@ static bool ShouldMatchFocusVisible( return false; } -static bool ShouldFocusRingBeVisible( - Element& aElement, int32_t aFlags, - const Maybe& 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, @@ -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); diff --git a/dom/tests/mochitest/general/test_focusrings.xhtml b/dom/tests/mochitest/general/test_focusrings.xhtml index 2895004d6204..efc2d9233863 100644 --- a/dom/tests/mochitest/general/test_focusrings.xhtml +++ b/dom/tests/mochitest/general/test_focusrings.xhtml @@ -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, @@ -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"); } @@ -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; diff --git a/layout/mathml/mathml.css b/layout/mathml/mathml.css index 26771cda1053..7bebcefd132d 100644 --- a/layout/mathml/mathml.css +++ b/layout/mathml/mathml.css @@ -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; } diff --git a/layout/style/res/forms.css b/layout/style/res/forms.css index 804ad87be6d3..628c0b18aebc 100644 --- a/layout/style/res/forms.css +++ b/layout/style/res/forms.css @@ -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 */ @@ -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; } @@ -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); } diff --git a/layout/style/res/html.css b/layout/style/res/html.css index b54e450b2e0f..55b2d7287f64 100644 --- a/layout/style/res/html.css +++ b/layout/style/res/html.css @@ -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; diff --git a/layout/style/res/ua.css b/layout/style/res/ua.css index bb5eaf197f6c..c147a9491146 100644 --- a/layout/style/res/ua.css +++ b/layout/style/res/ua.css @@ -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; } diff --git a/layout/svg/svg.css b/layout/svg/svg.css index 51fb0ce0f988..3901ac2e69aa 100644 --- a/layout/svg/svg.css +++ b/layout/svg/svg.css @@ -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; } diff --git a/modules/libpref/init/StaticPrefList.yaml b/modules/libpref/init/StaticPrefList.yaml index b6a4f7857a82..da2b07573217 100644 --- a/modules/libpref/init/StaticPrefList.yaml +++ b/modules/libpref/init/StaticPrefList.yaml @@ -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 @@ -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 diff --git a/servo/components/style/gecko/non_ts_pseudo_class_list.rs b/servo/components/style/gecko/non_ts_pseudo_class_list.rs index 7ed67725df0f..1fff5d19050c 100644 --- a/servo/components/style/gecko/non_ts_pseudo_class_list.rs +++ b/servo/components/style/gecko/non_ts_pseudo_class_list.rs @@ -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), diff --git a/servo/components/style/gecko/selector_parser.rs b/servo/components/style/gecko/selector_parser.rs index b619f58af60b..7010de45c7f9 100644 --- a/servo/components/style/gecko/selector_parser.rs +++ b/servo/components/style/gecko/selector_parser.rs @@ -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, } @@ -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"); } diff --git a/servo/components/style/gecko/wrapper.rs b/servo/components/style/gecko/wrapper.rs index 3a4bd8d759cb..80a2478e94c4 100644 --- a/servo/components/style/gecko/wrapper.rs +++ b/servo/components/style/gecko/wrapper.rs @@ -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 | diff --git a/testing/web-platform/meta/css/selectors/__dir__.ini b/testing/web-platform/meta/css/selectors/__dir__.ini index 248618a46f52..9e805240a90b 100644 --- a/testing/web-platform/meta/css/selectors/__dir__.ini +++ b/testing/web-platform/meta/css/selectors/__dir__.ini @@ -1,3 +1,2 @@ -prefs: [layout.css.focus-visible.enabled:true] lsan-disabled: true leak-threshold: [default:3276800, tab:460800] diff --git a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_001.html.ini b/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_001.html.ini deleted file mode 100644 index 46b3efee6b1a..000000000000 --- a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_001.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[drawFocusIfNeeded_001.html] - prefs: [layout.css.focus-visible.enabled:true] diff --git a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_004.html.ini b/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_004.html.ini deleted file mode 100644 index a896d7f50738..000000000000 --- a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_004.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[drawFocusIfNeeded_004.html] - prefs: [layout.css.focus-visible.enabled:true] diff --git a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_005.html.ini b/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_005.html.ini deleted file mode 100644 index 59f2b3ab11e6..000000000000 --- a/testing/web-platform/meta/html/canvas/element/manual/drawing-paths-to-the-canvas/drawFocusIfNeeded_005.html.ini +++ /dev/null @@ -1,2 +0,0 @@ -[drawFocusIfNeeded_005.html] - prefs: [layout.css.focus-visible.enabled:true]