Bug 1617342 - Don't treat scrollable boxes inside input elements as focusable. r=bzbarsky

The code in nsIFrame::IsFocusable works for non-number input elements, because
of the `IsRootOfNativeAnonymousSubtree` check, but doesn't work for
`<input type=number>`, as the scrollable area is wrapped in a flex container
(see nsNumberControlFrame::CreateAnonymousContent).

It used to work (kinda) before bug 981248 because of the weird focus-manager
redirection code to the inner <input> that was causing problems and which that
bug removed.

Make the check a bit more explicit, and add a test.

Differential Revision: https://phabricator.services.mozilla.com/D64432

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2020-02-27 18:44:33 +00:00
parent aadb5f6428
commit 4d9168305a
5 changed files with 72 additions and 2 deletions

View File

@ -666,6 +666,7 @@ skip-if = (toolkit == 'android') # Android: Bug 1465387
skip-if = (toolkit == 'android') # Android: Bug 1465387
[test_focus_shadow_dom_root.html]
[test_focus_shadow_dom.html]
[test_focus_scrollable_input.html]
[test_getAttribute_after_createAttribute.html]
[test_getElementById.html]
[test_getTranslationNodes.html]

View File

@ -0,0 +1,56 @@
<!doctype html>
<title>Test for bug 1617342</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<style>
input {
/* Make them small enough that any value overflows */
height: 10px;
width: 10px;
box-sizing: border-box;
font: 16px/1 monospace;
}
</style>
<input type="text" id="start" value="aaaaaaaaaa">
<input type="text" value="aaaaaaaaaa">
<input type="number" value="1111">
<input type="search" value="aaaaaaa">
<input type="url" value="https://fooooooooooo">
<input type="text" id="end" value="aaaaaaaaaa">
<script>
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(async function() {
// Enable Full Keyboard Access emulation on Mac.
if (navigator.platform.indexOf("Mac") === 0) {
await SpecialPowers.pushPrefEnv({"set": [["accessibility.tabfocus", 7]]});
}
const start = document.getElementById("start");
start.focus();
const end = document.getElementById("end");
is(document.activeElement, start, "Focus moved sanely");
let lastActiveElement = start;
let stack = [start];
do {
synthesizeKey("KEY_Tab");
isnot(document.activeElement, lastActiveElement, "Focus should've moved once per tab keypress");
lastActiveElement = document.activeElement;
stack.push(lastActiveElement);
} while (document.activeElement != end)
do {
let previous = stack.pop();
is(document.activeElement, previous, "Focus should've moved backwards as expected");
synthesizeKey("KEY_Tab", {shiftKey: true});
} while (stack.length);
SimpleTest.finish();
});
</script>

View File

@ -10195,6 +10195,7 @@ bool nsIFrame::IsFocusable(int32_t* aTabIndex, bool aWithMouse) {
// will be enough to make them keyboard scrollable.
nsIScrollableFrame* scrollFrame = do_QueryFrame(this);
if (scrollFrame &&
!scrollFrame->IsForTextControlWithNoScrollbars() &&
!scrollFrame->GetScrollStyles().IsHiddenInBothDirections() &&
!scrollFrame->GetScrollRange().IsEqualEdges(nsRect(0, 0, 0, 0))) {
// Scroll bars will be used for overflow

View File

@ -66,6 +66,8 @@ class ScrollFrameHelper : public nsIReflowCallback {
mozilla::ScrollStyles GetScrollStylesFromFrame() const;
mozilla::layers::OverscrollBehaviorInfo GetOverscrollBehaviorInfo() const;
bool IsForTextControlWithNoScrollbars() const;
// If a child frame was added or removed on the scrollframe,
// reload our child frame list.
// We need this if a scrollbar frame is recreated.
@ -728,8 +730,6 @@ class ScrollFrameHelper : public nsIReflowCallback {
bool HasBgAttachmentLocal() const;
mozilla::StyleDirection GetScrolledFrameDir() const;
bool IsForTextControlWithNoScrollbars() const;
// Ask APZ to smooth scroll to |aDestination|.
// This method does not clamp the destination; callers should clamp it to
// either the layout or the visual scroll range (APZ will happily smooth
@ -862,6 +862,9 @@ class nsHTMLScrollFrame : public nsContainerFrame,
mozilla::ScrollStyles GetScrollStyles() const override {
return mHelper.GetScrollStylesFromFrame();
}
bool IsForTextControlWithNoScrollbars() const final {
return mHelper.IsForTextControlWithNoScrollbars();
}
mozilla::layers::OverscrollBehaviorInfo GetOverscrollBehaviorInfo()
const final {
return mHelper.GetOverscrollBehaviorInfo();
@ -1330,6 +1333,9 @@ class nsXULScrollFrame final : public nsBoxFrame,
mozilla::ScrollStyles GetScrollStyles() const final {
return mHelper.GetScrollStylesFromFrame();
}
bool IsForTextControlWithNoScrollbars() const final {
return mHelper.IsForTextControlWithNoScrollbars();
}
mozilla::layers::OverscrollBehaviorInfo GetOverscrollBehaviorInfo()
const final {
return mHelper.GetOverscrollBehaviorInfo();

View File

@ -76,6 +76,12 @@ class nsIScrollableFrame : public nsIScrollbarMediator {
*/
virtual mozilla::ScrollStyles GetScrollStyles() const = 0;
/**
* Returns whether this scroll frame is for a text control element with no
* scrollbars (for <input>, basically).
*/
virtual bool IsForTextControlWithNoScrollbars() const = 0;
/**
* Get the overscroll-behavior styles.
*/