Skip to content

Commit

Permalink
Reland "Make elements outside of the fullscreen element inert"
Browse files Browse the repository at this point in the history
This is a reland of f3535b8

The IgnoreElementsOutsideFullscreenElement test was originally flaky,
due to changing textContent not reliably causing the accessibility tree
to be updated. See https://crbug.com/793078.

Original change's description:
> Make elements outside of the fullscreen element inert
>
> There is no spec for this, but an old open spec issue:
> whatwg/fullscreen#15
>
> Bug: 787867
> Change-Id: Icde796405fca96e910480aef6f0d6be835f7a27a
> Reviewed-on: https://chromium-review.googlesource.com/788052
> Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
> Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
> Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#520620}

Bug: 787867, 793078
Change-Id: I5ddc56a8783dce77acf02728afc9ed621145e222
Reviewed-on: https://chromium-review.googlesource.com/810584
Commit-Queue: Philip Jägenstedt <foolip@chromium.org>
Reviewed-by: Mounir Lamouri <mlamouri@chromium.org>
Reviewed-by: Dominic Mazzoni <dmazzoni@chromium.org>
Cr-Commit-Position: refs/heads/master@{#522771}
  • Loading branch information
foolip authored and Commit Bot committed Dec 8, 2017
1 parent 3b0721d commit 17dd72f
Show file tree
Hide file tree
Showing 7 changed files with 206 additions and 6 deletions.
108 changes: 108 additions & 0 deletions content/browser/accessibility/fullscreen_browsertest.cc
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
// Copyright 2017 The Chromium Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "base/logging.h"
#include "content/browser/accessibility/browser_accessibility.h"
#include "content/browser/accessibility/browser_accessibility_manager.h"
#include "content/browser/web_contents/web_contents_impl.h"
#include "content/public/test/browser_test_utils.h"
#include "content/public/test/content_browser_test.h"
#include "content/public/test/content_browser_test_utils.h"
#include "content/shell/browser/shell.h"
#include "content/test/accessibility_browser_test_utils.h"
#include "testing/gtest/include/gtest/gtest.h"

namespace content {

class AccessibilityFullscreenBrowserTest : public ContentBrowserTest {
public:
AccessibilityFullscreenBrowserTest() = default;
~AccessibilityFullscreenBrowserTest() override = default;

protected:
BrowserAccessibility* FindButton(BrowserAccessibility* node) {
if (node->GetRole() == ui::AX_ROLE_BUTTON)
return node;
for (unsigned i = 0; i < node->PlatformChildCount(); i++) {
if (BrowserAccessibility* button = FindButton(node->PlatformGetChild(i)))
return button;
}
return nullptr;
}

int CountLinks(BrowserAccessibility* node) {
if (node->GetRole() == ui::AX_ROLE_LINK)
return 1;
int links_in_children = 0;
for (unsigned i = 0; i < node->PlatformChildCount(); i++) {
links_in_children += CountLinks(node->PlatformGetChild(i));
}
return links_in_children;
}
};

namespace {

// FakeFullscreenDelegate simply stores the latest requested mod and reports it
// back, which is all that is required for the renderer to enter fullscreen.
class FakeFullscreenDelegate : public WebContentsDelegate {
public:
FakeFullscreenDelegate() = default;
~FakeFullscreenDelegate() override = default;

void EnterFullscreenModeForTab(WebContents*, const GURL&) override {
is_fullscreen_ = true;
}

void ExitFullscreenModeForTab(WebContents*) override {
is_fullscreen_ = false;
}

bool IsFullscreenForTabOrPending(const WebContents*) const override {
return is_fullscreen_;
}

private:
bool is_fullscreen_ = false;
DISALLOW_COPY_AND_ASSIGN(FakeFullscreenDelegate);
};

} // namespace

IN_PROC_BROWSER_TEST_F(AccessibilityFullscreenBrowserTest,
IgnoreElementsOutsideFullscreenElement) {
ASSERT_TRUE(embedded_test_server()->Start());

FakeFullscreenDelegate delegate;
shell()->web_contents()->SetDelegate(&delegate);

AccessibilityNotificationWaiter waiter(
shell()->web_contents(), ui::kAXModeComplete, ui::AX_EVENT_LOAD_COMPLETE);
GURL url(
embedded_test_server()->GetURL("/accessibility/fullscreen/links.html"));
NavigateToURL(shell(), url);
waiter.WaitForNotification();

WebContentsImpl* web_contents =
static_cast<WebContentsImpl*>(shell()->web_contents());
BrowserAccessibilityManager* manager =
web_contents->GetRootBrowserAccessibilityManager();

// Initially there are 3 links in the accessiblity tree.
EXPECT_EQ(3, CountLinks(manager->GetRoot()));

// Enter fullscreen by finding the button and performing the default action,
// which is to click it.
BrowserAccessibility* button = FindButton(manager->GetRoot());
ASSERT_NE(nullptr, button);
manager->DoDefaultAction(*button);

// Upon entering fullscreen, the page will change the button text to "Done".
WaitForAccessibilityTreeToContainNodeWithName(web_contents, "Done");

// Now, the two links outside of the fullscreen element are gone.
EXPECT_EQ(1, CountLinks(manager->GetRoot()));
}

} // namespace content
5 changes: 4 additions & 1 deletion content/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,7 @@ test("content_browsertests") {
"../browser/accessibility/dump_accessibility_browsertest_base.h",
"../browser/accessibility/dump_accessibility_events_browsertest.cc",
"../browser/accessibility/dump_accessibility_tree_browsertest.cc",
"../browser/accessibility/fullscreen_browsertest.cc",
"../browser/accessibility/hit_testing_browsertest.cc",
"../browser/accessibility/site_per_process_accessibility_browsertest.cc",
"../browser/accessibility/snapshot_ax_tree_browsertest.cc",
Expand Down Expand Up @@ -1888,7 +1889,9 @@ test("content_unittests") {
# Screen capture unit tests.
if (is_linux || is_mac || is_win) {
deps += [ "//third_party/libyuv" ]
sources += [ "../browser/media/capture/web_contents_video_capture_device_unittest.cc" ]
sources += [
"../browser/media/capture/web_contents_video_capture_device_unittest.cc",
]
if (use_aura) {
sources += [ "../browser/media/capture/cursor_renderer_aura_unittest.cc" ]
}
Expand Down
19 changes: 19 additions & 0 deletions content/test/data/accessibility/fullscreen/links.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
<!DOCTYPE html>
<a href="/">link before target</a>
<div id="target">
<button>Enter fullscreen</button>
<a href="/">link inside target</a>
</div>
<a href="/">link after target</a>
<script>
const target = document.getElementById('target');

const button = document.querySelector('button');
button.addEventListener('click', function() {
target.webkitRequestFullscreen();
});

document.onwebkitfullscreenchange = function() {
button.setAttribute('aria-label', 'Done');
}
</script>
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
CONSOLE WARNING: line 22: The behavior that Selection.addRange() merges existing Range and the specified Range was removed. See https://www.chromestatus.com/features/6680566019653632 for more details.
Pass if there is no crash.
Click anywhere to test manually.
56 changes: 56 additions & 0 deletions third_party/WebKit/LayoutTests/fullscreen/keyboard-focus.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
<!DOCTYPE html>
<title>Elements not contained by the fullscreen element are inert</title>
<script src="../resources/testharness.js"></script>
<script src="../resources/testharnessreport.js"></script>
<script src="../shadow-dom/resources/focus-utils.js"></script>
<script src="trusted-click.js"></script>
<div id="log"></div>
<div id="before">
<input>
</div>
<div id="target">
<input>
<input>
</div>
<div id="after">
<input>
</div>
<script>
// The important side effect of inertness is that focus moves away from an
// element that becomes inert, and that when in fullscreen it's not possible to
// move focus out of the fullscreen element.
//
// TODO(foolip): This is not yet per spec, so change the spec:
// https://github.com/whatwg/fullscreen/issues/15
async_test(t => {
const target = document.getElementById("target");
const beforeInput = document.querySelector("#before input");
const targetInputs = target.querySelectorAll("input");

// Initial focus is body, cycle to first input.
assert_equals(document.activeElement, document.body, 'active element #1');
navigateFocusForward();
assert_equals(document.activeElement, beforeInput, 'active element #2');

document.onfullscreenchange = t.step_func_done(() => {
// Entering fullscreen moved the focus back to body.
assert_equals(document.activeElement, document.body);

// Cycling focus should now skip the inert elements.
navigateFocusForward();
assert_equals(document.activeElement, targetInputs[0], 'active element #3');
navigateFocusForward();
assert_equals(document.activeElement, targetInputs[1], 'active element #4');
navigateFocusForward();
assert_equals(document.activeElement, targetInputs[0], 'active element #5');

// Cycling focus in reverse should also skip the inert elements.
navigateFocusBackward();
assert_equals(document.activeElement, targetInputs[1], 'active element #6');
navigateFocusBackward();
assert_equals(document.activeElement, targetInputs[0], 'active element #7');
});

trusted_request(t, target);
});
</script>
18 changes: 14 additions & 4 deletions third_party/WebKit/Source/core/dom/Node.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@
#include "core/frame/LocalFrameClient.h"
#include "core/frame/LocalFrameView.h"
#include "core/frame/UseCounter.h"
#include "core/fullscreen/Fullscreen.h"
#include "core/html/HTMLDialogElement.h"
#include "core/html/HTMLFrameOwnerElement.h"
#include "core/html/HTMLSlotElement.h"
Expand Down Expand Up @@ -914,10 +915,19 @@ bool Node::IsInert() const {

DCHECK(!ChildNeedsDistributionRecalc());

const HTMLDialogElement* dialog = GetDocument().ActiveModalDialog();
if (dialog && this != GetDocument() &&
!FlatTreeTraversal::ContainsIncludingPseudoElement(*dialog, *this)) {
return true;
if (this != GetDocument()) {
// TODO(foolip): When fullscreen uses top layer, this can be simplified to
// just look at the topmost element in top layer. https://crbug.com/240576.
// Note: It's currently appropriate that a modal dialog element takes
// precedence over a fullscreen element, because it will be rendered on top,
// but with fullscreen merged into top layer that will no longer be true.
const Element* modal_element = GetDocument().ActiveModalDialog();
if (!modal_element)
modal_element = Fullscreen::FullscreenElementFrom(GetDocument());
if (modal_element && !FlatTreeTraversal::ContainsIncludingPseudoElement(
*modal_element, *this)) {
return true;
}
}

if (RuntimeEnabledFeatures::InertAttributeEnabled()) {
Expand Down
5 changes: 5 additions & 0 deletions third_party/WebKit/Source/core/fullscreen/Fullscreen.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -903,6 +903,11 @@ void Fullscreen::FullscreenElementChanged(Element* old_element,

// TODO(foolip): This should not call |UpdateStyleAndLayoutTree()|.
GetDocument()->UpdateStyleAndLayoutTree();

// Any element not contained by the fullscreen element is inert (see
// |Node::IsInert()|), so changing the fullscreen element will typically
// change the inertness of most elements. Clear the entire cache.
GetDocument()->ClearAXObjectCache();
}

void Fullscreen::Trace(blink::Visitor* visitor) {
Expand Down

0 comments on commit 17dd72f

Please sign in to comment.