Commit 9bee9146 authored by Vladimir Levin's avatar Vladimir Levin Committed by Commit Bot

SubtreeVisibility: don't unlock in activation (except find-in-page).

This patch removes unlocking for activation. This is instead replaced
with scrolling the element into view for the auto case.

Find-in-page still activates, since it needs further testing and because
it needs to work with hidden-matchable as well as auto.

R=chrishtr@chromium.org, aleventhal@chromium.org

Change-Id: Ice3e488281da6e55bac1451c596e648aaa2b440f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2130680
Commit-Queue: vmpstr <vmpstr@chromium.org>
Reviewed-by: default avatarChris Harrelson <chrishtr@chromium.org>
Reviewed-by: default avatarAaron Leventhal <aleventhal@chromium.org>
Cr-Commit-Position: refs/heads/master@{#755379}
parent 14648614
...@@ -383,14 +383,10 @@ void DisplayLockContext::CommitForActivationWithSignal( ...@@ -383,14 +383,10 @@ void DisplayLockContext::CommitForActivationWithSignal(
RecordActivationReason(document_, reason); RecordActivationReason(document_, reason);
// TODO(vmpstr): This should eventually never unlock, but each reason needs to // TODO(vmpstr): This should be removed after find-in-page doesn't rely on
// be tested and considered, so this is being done in parts. // this.
if (reason == DisplayLockActivationReason::kScrollIntoView || if (reason == DisplayLockActivationReason::kFindInPage)
reason == DisplayLockActivationReason::kScriptFocus || Unlock();
reason == DisplayLockActivationReason::kUserFocus)
return;
Unlock();
} }
void DisplayLockContext::NotifyIsIntersectingViewport() { void DisplayLockContext::NotifyIsIntersectingViewport() {
......
...@@ -19,7 +19,7 @@ class CORE_EXPORT DisplayLockUtilities { ...@@ -19,7 +19,7 @@ class CORE_EXPORT DisplayLockUtilities {
public: public:
// This class forces updates on display locks from the given node up the // This class forces updates on display locks from the given node up the
// ancestor chain until the local frame root. // ancestor chain until the local frame root.
class ScopedChainForcedUpdate { class CORE_EXPORT ScopedChainForcedUpdate {
DISALLOW_COPY_AND_ASSIGN(ScopedChainForcedUpdate); DISALLOW_COPY_AND_ASSIGN(ScopedChainForcedUpdate);
public: public:
......
...@@ -3401,15 +3401,17 @@ bool AXObject::InternalSetAccessibilityFocusAction() { ...@@ -3401,15 +3401,17 @@ bool AXObject::InternalSetAccessibilityFocusAction() {
bool AXObject::OnNativeScrollToMakeVisibleAction() const { bool AXObject::OnNativeScrollToMakeVisibleAction() const {
Node* node = GetNode(); Node* node = GetNode();
if (!node) if (!node || !node->isConnected())
return false; return false;
if (Element* locked_ancestor =
DisplayLockUtilities::NearestLockedInclusiveAncestor(*node)) { // Node might not have a LayoutObject due to the fact that it is in a locked
locked_ancestor->ActivateDisplayLockIfNeeded( // subtree. Force the update to create the LayoutObject (and update position
DisplayLockActivationReason::kAccessibility); // information) for this node.
} DisplayLockUtilities::ScopedChainForcedUpdate scoped_force_update(node);
GetDocument()->UpdateStyleAndLayout(DocumentUpdateReason::kDisplayLock);
LayoutObject* layout_object = node->GetLayoutObject(); LayoutObject* layout_object = node->GetLayoutObject();
if (!layout_object || !node->isConnected()) if (!layout_object)
return false; return false;
PhysicalRect target_rect(layout_object->AbsoluteBoundingBoxRect()); PhysicalRect target_rect(layout_object->AbsoluteBoundingBoxRect());
layout_object->ScrollRectToVisible( layout_object->ScrollRectToVisible(
......
<!doctype HTML> <!doctype HTML>
<html> <html>
<meta charset="utf8"> <meta charset="utf8">
<title>Subtree Visibility: accessibility focus</title> <title>Subtree Visibility: accessibility scrollToMakeVisible()</title>
<link rel="author" title="Rakina Zata Amni" href="mailto:rakina@chromium.org"> <link rel="author" title="Rakina Zata Amni" href="mailto:rakina@chromium.org">
<link rel="help" href="https://github.com/WICG/display-locking"> <link rel="help" href="https://github.com/WICG/display-locking">
<link rel="match" href="container-ref.html">
<meta name="assert" content="subtree-visibility auto subtrees are exposed by accessibility scrollToMakeVisible"> <meta name="assert" content="subtree-visibility auto subtrees are exposed by accessibility scrollToMakeVisible">
<script src="/resources/testharness.js"></script> <script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script> <script src="/resources/testharnessreport.js"></script>
<style> <style>
.spacer {
height: 3000px;
}
.auto { .auto {
subtree-visibility: auto; subtree-visibility: auto;
} }
</style> </style>
<div id="hidden" style="subtree-visibility: auto"> <div class=spacer></div>
<div id="hidden" class=auto>
foo foo
<div id="child" tabindex="0"> <div id="child" tabindex="0">
bar bar
</div> </div>
</div> </div>
<div class=spacer></div>
<script> <script>
function axElementById(id) { function axElementById(id) {
...@@ -35,14 +39,21 @@ async_test(async(t) => { ...@@ -35,14 +39,21 @@ async_test(async(t) => {
axElementById("child").scrollToMakeVisible(); axElementById("child").scrollToMakeVisible();
// Wait for the next frame for the ax object to be recreated. // If this is frame 1, then during frame 2, we will determine that we're
requestAnimationFrame(() => { // visible and issue notifications of viewport intersection. However, the
requestAnimationFrame(() => { // visibility notification is delayed to frame 3 past the rAF callbacks. So,
axHidden = axElementById("hidden"); // check the result in frame 4.
t.step(() => { assert_equals(axHidden.childrenCount, 2, "Child count after activation"); }); requestAnimationFrame(() => { // Frame 2.
t.done(); requestAnimationFrame(() => { // Frame 3.
requestAnimationFrame(() => { // Frame 4.
axHidden = axElementById("hidden");
t.step(() => {
assert_equals(axHidden.childrenCount, 2, "Child count after activation");
});
t.done();
});
}); });
}); });
}, "Accessiblility press() causes activatable hidden tree to activate"); }, "Accessiblility scrollToMakeVisible() causes activatable hidden tree to activate");
</script> </script>
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment