Magic-Mirror/gecko-dev
1
0
Fork 0
mirror of https://github.com/mozilla/gecko-dev.git synced 2025-01-14 22:13:31 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Bug 1629096 - Always honor scroll-margin / scroll-padding from nsFocusManager. r=masayuki

Browse Source 
We should always do this, otherwise stuff may not end up being visible which is
not acceptable for focus navigation.

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

--HG--
extra : moz-landing-system : lando
This commit is contained in:
Emilio Cobos Álvarez 2020-04-13 11:34:48 +00:00
parent 9e9b510199
commit 142b7d3c57
13 changed files with 107 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
browser/components/customizableui/PanelMultiView.jsm
View File

@ -1871,8 +1871,8 @@ var PanelView = class extends AssociatedToNode {
focusSelectedElement(byKey = false) {
let selected = this.selectedElement;
if (selected) {
let flag = byKey ? "FLAG_BYKEY" : "FLAG_BYELEMENTFOCUS";
Services.focus.setFocus(selected, Services.focus[flag]);
let flag = byKey ? Services.focus.FLAG_BYKEY : 0;
Services.focus.setFocus(selected, flag);
}
}

24
dom/base/Element.cpp
View File

@ -362,25 +362,25 @@ int32_t Element::TabIndex() {
void Element::Focus(const FocusOptions& aOptions, CallerType aCallerType,
ErrorResult& aError) {
nsFocusManager* fm = nsFocusManager::GetFocusManager();
if (!fm) {
return;
}
// Also other browsers seem to have the hack to not re-focus (and flush) when
// the element is already focused.
// Until https://github.com/whatwg/html/issues/4512 is clarified, we'll
// maintain interoperatibility by not re-focusing, independent of aOptions.
// I.e., `focus({ preventScroll: true})` followed by `focus( { preventScroll:
// false })` won't re-focus.
if (fm) {
if (fm->CanSkipFocus(this)) {
fm->NeedsFlushBeforeEventHandling(this);
} else {
uint32_t fmFlags =
nsIFocusManager::FLAG_BYELEMENTFOCUS |
nsFocusManager::FocusOptionsToFocusManagerFlags(aOptions);
if (aCallerType == CallerType::NonSystem) {
fmFlags = nsIFocusManager::FLAG_NONSYSTEMCALLER | fmFlags;
}
aError = fm->SetFocus(this, fmFlags);
}
if (fm->CanSkipFocus(this)) {
fm->NeedsFlushBeforeEventHandling(this);
return;
}
uint32_t fmFlags =
nsFocusManager::FocusOptionsToFocusManagerFlags(aOptions);
if (aCallerType == CallerType::NonSystem) {
fmFlags |= nsIFocusManager::FLAG_NONSYSTEMCALLER;
}
aError = fm->SetFocus(this, fmFlags);
}
void Element::SetTabIndex(int32_t aTabIndex, mozilla::ErrorResult& aError) {

16
dom/base/nsFocusManager.cpp
View File

@ -2596,16 +2596,14 @@ void nsFocusManager::FireFocusOrBlurEvent(EventMessage aEventMessage,
void nsFocusManager::ScrollIntoView(PresShell* aPresShell, nsIContent* aContent,
uint32_t aFlags) {
// if the noscroll flag isn't set, scroll the newly focused element into view
if (!(aFlags & FLAG_NOSCROLL)) {
ScrollFlags scrollFlags = ScrollFlags::ScrollOverflowHidden;
if (!(aFlags & FLAG_BYELEMENTFOCUS)) {
scrollFlags |= ScrollFlags::IgnoreMarginAndPadding;
}
aPresShell->ScrollContentIntoView(
aContent, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible), scrollFlags);
if (aFlags & FLAG_NOSCROLL) {
return;
}
// If the noscroll flag isn't set, scroll the newly focused element into view.
aPresShell->ScrollContentIntoView(
aContent, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollFlags::ScrollOverflowHidden);
}
void nsFocusManager::RaiseWindow(nsPIDOMWindowOuter* aWindow,

1
dom/base/test/mochitest.ini
View File

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

69
dom/base/test/test_focus_scroll_padding_tab.html Normal file
View File

@ -0,0 +1,69 @@
<!doctype html>
<title>Test for bug 1617342</title>
<script src="/tests/SimpleTest/SimpleTest.js"></script>
<script src="/tests/SimpleTest/EventUtils.js"></script>
<style>
:root {
scroll-padding: 100px 0;
}
body {
margin: 0;
}
#fixed-footer {
height: 100px;
position: fixed;
bottom: 0;
left: 0;
width: 100%;
background: green;
}
#fixed-header {
height: 100px;
position: fixed;
top: 0;
left: 0;
width: 100%;
background: green;
}
.padding {
height: 100vh;
}
</style>
<div id="fixed-header"></div>
<div class="padding"></div>
<input>
<div class="padding"></div>
<input id="end">
<div class="padding"></div>
<div id="fixed-footer"></div>
<script>
SimpleTest.waitForExplicitFinish();
SimpleTest.waitForFocus(async function() {
let stack = [];
let end = document.getElementById("end");
let lastActiveElement = document.activeElement;
do {
synthesizeKey("KEY_Tab");
is(document.activeElement.tagName, "INPUT", "Should focus inputs only, there's nothing else");
isnot(document.activeElement, lastActiveElement, "Focus should've moved once per tab keypress");
let rect = document.activeElement.getBoundingClientRect();
ok(rect.top >= 100, "Should not be covered by top bar");
ok(rect.bottom >= 100, "Should not be covered by bottom bar");
lastActiveElement = document.activeElement;
stack.push(lastActiveElement);
} while (document.activeElement != end)
do {
let previous = stack.pop();
let rect = document.activeElement.getBoundingClientRect();
ok(rect.top >= 100, "Should not be covered by top bar");
ok(rect.bottom >= 100, "Should not be covered by bottom bar");
is(document.activeElement, previous, "Focus should've moved backwards as expected");
synthesizeKey("KEY_Tab", {shiftKey: true});
} while (stack.length);
SimpleTest.finish();
});
</script>

10
dom/events/EventStateManager.cpp
View File

@ -1063,13 +1063,9 @@ bool EventStateManager::LookForAccessKeyAndExecute(
if (shouldActivate) {
focusChanged =
element->PerformAccesskey(shouldActivate, aIsTrustedEvent);
} else {
nsIFocusManager* fm = nsFocusManager::GetFocusManager();
if (fm) {
fm->SetFocus(element, nsIFocusManager::FLAG_BYKEY |
nsIFocusManager::FLAG_BYELEMENTFOCUS);
focusChanged = true;
}
} else if (nsFocusManager* fm = nsFocusManager::GetFocusManager()) {
fm->SetFocus(element, nsIFocusManager::FLAG_BYKEY);
focusChanged = true;
}
if (focusChanged && aIsTrustedEvent) {

4
dom/html/HTMLLabelElement.cpp
View File

@ -128,8 +128,7 @@ nsresult HTMLLabelElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
// Only set focus on the first click of multiple clicks to prevent
// to prevent immediate de-focus.
if (mouseEvent->mClickCount <= 1) {
nsIFocusManager* fm = nsFocusManager::GetFocusManager();
if (fm) {
if (nsFocusManager* fm = nsFocusManager::GetFocusManager()) {
// Use FLAG_BYMOVEFOCUS here so that the label is scrolled to.
// Also, within HTMLInputElement::PostHandleEvent, inputs will
// be selected only when focused via a key or when the navigation
@ -145,7 +144,6 @@ nsresult HTMLLabelElement::PostHandleEvent(EventChainPostVisitor& aVisitor) {
MouseEvent_Binding::MOZ_SOURCE_TOUCH);
fm->SetFocus(content,
nsIFocusManager::FLAG_BYMOVEFOCUS |
nsIFocusManager::FLAG_BYELEMENTFOCUS |
(byMouse ? nsIFocusManager::FLAG_BYMOUSE : 0) |
(byTouch ? nsIFocusManager::FLAG_BYTOUCH : 0));
}

1
dom/html/HTMLLegendElement.cpp
View File

@ -91,7 +91,6 @@ void HTMLLegendElement::Focus(const FocusOptions& aOptions,
aError = fm->MoveFocus(
nullptr, this, nsIFocusManager::MOVEFOCUS_FORWARD,
nsIFocusManager::FLAG_NOPARENTFRAME |
nsIFocusManager::FLAG_BYELEMENTFOCUS |
nsFocusManager::FocusOptionsToFocusManagerFlags(aOptions),
getter_AddRefs(result));
}

11
dom/html/nsGenericHTMLElement.cpp
View File

@ -2411,14 +2411,15 @@ bool nsGenericHTMLElement::PerformAccesskey(bool aKeyCausesActivation,
// It's hard to say what HTML4 wants us to do in all cases.
bool focused = true;
nsFocusManager* fm = nsFocusManager::GetFocusManager();
if (fm) {
fm->SetFocus(this, nsIFocusManager::FLAG_BYKEY |
nsIFocusManager::FLAG_BYELEMENTFOCUS);
if (nsFocusManager* fm = nsFocusManager::GetFocusManager()) {
fm->SetFocus(this, nsIFocusManager::FLAG_BYKEY);
// Return true if the element became the current focus within its window.
//
// FIXME(emilio): Shouldn't this check `window->GetFocusedElement() == this`
// based on the above comment?
nsPIDOMWindowOuter* window = OwnerDoc()->GetWindow();
focused = (window && window->GetFocusedElement());
focused = window && window->GetFocusedElement();
}
if (aKeyCausesActivation) {

13
dom/interfaces/base/nsIFocusManager.idl
View File

@ -167,8 +167,7 @@ interface nsIFocusManager : nsISupports
const unsigned long FLAG_RAISE = 1;
/**
* Do not scroll the element to focus into view. If FLAG_BYELEMENTFOCUS is
* set too, the latter is ignored.
* Do not scroll the element to focus into view.
*/
const unsigned long FLAG_NOSCROLL = 2;
@ -228,16 +227,6 @@ interface nsIFocusManager : nsISupports
*/
const unsigned long FLAG_BYTOUCH = 0x200000;
/**
* Focus is changing due to a Element.focus() call.
* This is used to distinguish the Element.focus() call from other focus
* functionalities since we need to factor scroll-margin and scroll-padding
* values into the position where we scroll to the element by the
* Element.focus() call.
* NOTE: This flag is ignored if FLAG_NOSCROLL is set too.
*/
const unsigned long FLAG_BYELEMENTFOCUS = 0x400000;
/*
* Focus is changing due to a long press operation by touch or mouse.
*/

3
layout/base/PresShell.cpp
View File

@ -8758,8 +8758,7 @@ void PresShell::EventHandler::GetCurrentItemAndPositionForElement(
nsCOMPtr<nsIContent> focusedContent = aFocusedElement;
MOZ_KnownLive(mPresShell)
->ScrollContentIntoView(focusedContent, ScrollAxis(), ScrollAxis(),
ScrollFlags::ScrollOverflowHidden |
ScrollFlags::IgnoreMarginAndPadding);
ScrollFlags::ScrollOverflowHidden);
nsPresContext* presContext = GetPresContext();

8
toolkit/components/find/nsWebBrowserFind.cpp
View File

@ -742,13 +742,11 @@ nsresult nsWebBrowserFind::OnFind(nsPIDOMWindowOuter* aFoundWindow) {
ClearFrameSelection(lastFocusedWindow);
}
nsFocusManager* fm = nsFocusManager::GetFocusManager();
if (fm) {
if (nsFocusManager* fm = nsFocusManager::GetFocusManager()) {
// get the containing frame and focus it. For top-level windows, the right
// window should already be focused.
RefPtr<Element> frameElement = aFoundWindow->GetFrameElementInternal();
if (frameElement) {
fm->SetFocus(frameElement, nsIFocusManager::FLAG_BYELEMENTFOCUS);
if (RefPtr<Element> frameElement = aFoundWindow->GetFrameElementInternal()) {
fm->SetFocus(frameElement, 0);
}
mLastFocusedWindow = do_GetWeakReference(aFoundWindow);

3
toolkit/components/satchel/nsFormFillController.cpp
View File

@ -369,8 +369,7 @@ nsFormFillController::SetPopupOpen(bool aPopupOpen) {
presShell->ScrollContentIntoView(
content, ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollAxis(kScrollMinimum, WhenToScroll::IfNotVisible),
ScrollFlags::ScrollOverflowHidden |
ScrollFlags::IgnoreMarginAndPadding);
ScrollFlags::ScrollOverflowHidden);
// mFocusedPopup can be destroyed after ScrollContentIntoView, see bug
// 420089
if (mFocusedPopup) {