Skip to content

Commit

Permalink
Revert "Fenced frames: Add accessibility tree test for fenced frames."
Browse files Browse the repository at this point in the history
This reverts commit 8d3202a.

Reason for revert: New test failing  All/DumpAccessibilityTreeFencedFrameMPArchTest.AccessibilityFencedFrameScrollable/blink 

Bug:1324389

Original change's description:
> Fenced frames: Add accessibility tree test for fenced frames.
>
> Builds off of a fenced frame test that was in place, but was only
> testing iframes rather than fenced frames. The test will ensure that
> AXObject::Init is hit (which was the function that had the check being
> hit before).
>
> Bug: 1316348
> Change-Id: I7265f04f06e269aee405e416ff3b31b540c8f33c
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3627914
> Reviewed-by: Dominic Farolino <dom@chromium.org>
> Commit-Queue: Liam Brady <lbrady@google.com>
> Reviewed-by: Aaron Leventhal <aleventhal@chromium.org>
> Cr-Commit-Position: refs/heads/main@{#1001209}

Bug: 1316348
Change-Id: If181fa2d8efae846c5055f21f39af5b7b80e77cf
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3639423
Commit-Queue: Yuzu Saijo <yuzus@chromium.org>
Bot-Commit: Rubber Stamper <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Yuzu Saijo <yuzus@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1001847}
  • Loading branch information
Yuzu Saijo authored and Chromium LUCI CQ committed May 11, 2022
1 parent e5bbd6a commit f825882
Show file tree
Hide file tree
Showing 11 changed files with 20 additions and 149 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -294,9 +294,8 @@ void DumpAccessibilityTestBase::RunTestForPlatform(
test_helper_.LoadExpectationFile(expected_file);

// Get the test URL.
GURL url(embedded_test_server()->GetURL(
"a.test",
"/" + std::string(file_dir) + "/" + file_path.BaseName().MaybeAsASCII()));
GURL url(embedded_test_server()->GetURL("/" + std::string(file_dir) + "/" +
file_path.BaseName().MaybeAsASCII()));
WebContentsImpl* web_contents = GetWebContents();

if (enable_accessibility_after_navigating_ &&
Expand Down Expand Up @@ -584,12 +583,4 @@ DumpAccessibilityTestBase::FindNodeByHTMLAttributeInSubtree(
return nullptr;
}

void DumpAccessibilityTestBase::UseHttpsTestServer() {
https_test_server_ = std::make_unique<net::EmbeddedTestServer>(
net::EmbeddedTestServer::TYPE_HTTPS);
https_test_server_.get()->AddDefaultHandlers(GetTestDataFilePath());
https_test_server_.get()->SetSSLConfig(
net::EmbeddedTestServer::CERT_TEST_NAMES);
}

} // namespace content
Original file line number Diff line number Diff line change
Expand Up @@ -160,20 +160,6 @@ class DumpAccessibilityTestBase
// Wait for default action, expected text and then end of test signal.
void WaitForFinalTreeContents();

// Creates a new secure test server that can be used in place of the default
// HTTP embedded_test_server defined in BrowserTestBase. The new test server
// can then be retrieved using the same embedded_test_server() method used
// to get the BrowserTestBase HTTP server.
void UseHttpsTestServer();

// This will return either the https test server or the
// default one specified in BrowserTestBase, depending on if an https test
// server was created by calling UseHttpsTestServer().
net::EmbeddedTestServer* embedded_test_server() {
return (https_test_server_) ? https_test_server_.get()
: BrowserTestBase::embedded_test_server();
}

private:
BrowserAccessibility* FindNodeInSubtree(BrowserAccessibility& node,
const std::string& name) const;
Expand All @@ -195,11 +181,6 @@ class DumpAccessibilityTestBase
}

bool has_performed_default_actions_ = false;

// Secure test server, isn't created by default. Needs to be
// created using UseHttpsTestServer() and then called with
// embedded_test_server().
std::unique_ptr<net::EmbeddedTestServer> https_test_server_;
};

} // namespace content
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2028,73 +2028,9 @@ IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
RunHtmlTest(FILE_PATH_LITERAL("iframe-empty-positioned.html"));
}

class DumpAccessibilityTreeFencedFrameShadowDOMTest
: public DumpAccessibilityTreeTest {
protected:
DumpAccessibilityTreeFencedFrameShadowDOMTest() {
feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kFencedFrames,
{{"implementation_type", "shadow_dom"}}},
{features::kPrivacySandboxAdsAPIsOverride, {}}},
{/* disabled_features */});

UseHttpsTestServer();
}

~DumpAccessibilityTreeFencedFrameShadowDOMTest() override {
// Ensure that the feature lists are destroyed in the same order they
// were created in.
scoped_feature_list_.Reset();
feature_list_.Reset();
}

private:
base::test::ScopedFeatureList feature_list_;
};

INSTANTIATE_TEST_SUITE_P(
All,
DumpAccessibilityTreeFencedFrameShadowDOMTest,
::testing::ValuesIn(ui::AXInspectTestHelper::TreeTestPasses()),
DumpAccessibilityTreeTestPassToString());

IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeFencedFrameShadowDOMTest,
AccessibilityFencedFrameScrollable) {
RunHtmlTest(FILE_PATH_LITERAL("fencedframe-scrollable-shadowdom.html"));
}

class DumpAccessibilityTreeFencedFrameMPArchTest
: public DumpAccessibilityTreeTest {
protected:
DumpAccessibilityTreeFencedFrameMPArchTest() {
feature_list_.InitWithFeaturesAndParameters(
{{blink::features::kFencedFrames, {{"implementation_type", "mparch"}}},
{features::kPrivacySandboxAdsAPIsOverride, {}}},
{/* disabled_features */});

UseHttpsTestServer();
}

~DumpAccessibilityTreeFencedFrameMPArchTest() override {
// Ensure that the feature lists are destroyed in the same order they
// were created in.
scoped_feature_list_.Reset();
feature_list_.Reset();
}

private:
base::test::ScopedFeatureList feature_list_;
};

INSTANTIATE_TEST_SUITE_P(
All,
DumpAccessibilityTreeFencedFrameMPArchTest,
::testing::ValuesIn(ui::AXInspectTestHelper::TreeTestPasses()),
DumpAccessibilityTreeTestPassToString());

IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeFencedFrameMPArchTest,
IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
AccessibilityFencedFrameScrollable) {
RunHtmlTest(FILE_PATH_LITERAL("fencedframe-scrollable-mparch.html"));
RunHtmlTest(FILE_PATH_LITERAL("fencedframe-scrollable.html"));
}

IN_PROC_BROWSER_TEST_P(DumpAccessibilityTreeTest,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
STATE-CHANGE:BUSY:TRUE role=ROLE_DOCUMENT_WEB name='Starting title' BUSY,ENABLED,FOCUSABLE,FOCUSED,SENSITIVE,SHOWING,VISIBLE
STATE-CHANGE:EXPANDED:FALSE role=ROLE_STATIC name='Stop' ENABLED,SENSITIVE
TEXT-INSERT (start=40 length=52 'this-will-be-converted-to-a-same-document-navigation') role=ROLE_ENTRY name='Enter URL' EDITABLE,ENABLED,FOCUSABLE,SENSITIVE,SHOWING,SINGLE-LINE,VISIBLE,SELECTABLE-TEXT
TEXT-REMOVE (start=40 length=14 'navigation-api') role=ROLE_ENTRY name='Enter URL' EDITABLE,ENABLED,FOCUSABLE,SENSITIVE,SHOWING,SINGLE-LINE,VISIBLE,SELECTABLE-TEXT
TEXT-INSERT (start=43 length=52 'this-will-be-converted-to-a-same-document-navigation') role=ROLE_ENTRY name='Enter URL' EDITABLE,ENABLED,FOCUSABLE,SENSITIVE,SHOWING,SINGLE-LINE,VISIBLE,SELECTABLE-TEXT
TEXT-REMOVE (start=43 length=14 'navigation-api') role=ROLE_ENTRY name='Enter URL' EDITABLE,ENABLED,FOCUSABLE,SENSITIVE,SHOWING,SINGLE-LINE,VISIBLE,SELECTABLE-TEXT
=== Start Continuation ===
CHILDREN-CHANGED:ADD index:0 CHILD:(role=ROLE_PARAGRAPH) role=ROLE_LANDMARK ENABLED,SENSITIVE,SHOWING,VISIBLE
CHILDREN-CHANGED:REMOVE index:0 CHILD:(role=ROLE_PARAGRAPH) role=ROLE_LANDMARK ENABLED,SENSITIVE,SHOWING,VISIBLE
Expand Down

This file was deleted.

This file was deleted.

This file was deleted.

10 changes: 10 additions & 0 deletions content/test/data/accessibility/html/fencedframe-scrollable.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
<!--
@BLINK-ALLOW:scrollable=*
-->
<!DOCTYPE html>
<html style="width:100px; height:100px;">
<body>
<iframe style="width:200px; height: 200px;" aria-label="Scrollable iframe" src="frame/visible_text.html">
</iframe>
</body>
</html>

This file was deleted.

17 changes: 4 additions & 13 deletions third_party/blink/renderer/modules/accessibility/ax_object.cc
Original file line number Diff line number Diff line change
Expand Up @@ -647,27 +647,18 @@ void AXObject::Init(AXObject* parent) {
if (parent_ && parent_->RoleValue() == ax::mojom::blink::Role::kIframe &&
RoleValue() != ax::mojom::blink::Role::kDocument) {
// A frame/iframe can only have a document child.
// Make an exception for ShadowDOM based fenced frames. While they have
// the same role as a regular IFrame, they will have an inner iframe
// be the child of the outer iframe, rather than a document. This
// behavior is expected and the exception is carved out here.
if (!blink::features::IsFencedFramesEnabled() ||
!blink::features::IsFencedFramesShadowDOMBased() ||
!IsA<HTMLFencedFrameElement>(parent_->GetNode())) {
// Exception for now: shadow DOM fenced frame.
// TODO(crbug.com/1316348): see if AXNodeObject::AddNodeChildren() needs
// to change for fenced frames similar to iframes and whether this change
// would then still be necessary.
NOTREACHED() << "An iframe can only have a document child."
<< "\n* Child = " << ToString(true, true)
<< "\n* Parent = " << parent_->ToString(true, true);
}
}

if (blink::features::IsFencedFramesEnabled() &&
blink::features::IsFencedFramesShadowDOMBased() && parent_ &&
IsA<HTMLFencedFrameElement>(parent_->GetNode()) &&
RoleValue() != ax::mojom::blink::Role::kIframe) {
NOTREACHED() << "A ShadowDOM fenced frame must have an iframe child."
<< "\n* Child = " << ToString(true, true)
<< "\n* Parent = " << parent_->ToString(true, true);
}
#endif
}

Expand Down

0 comments on commit f825882

Please sign in to comment.