Commit 86ec2995 authored by Stephane Zermatten's avatar Stephane Zermatten Committed by Commit Bot

[Autofill Assistant] Control whether the on_top filter can scroll.

This change extends the on_top filter to allow controlling:
 - whether it should call scrollIntoViewIfNeeded
 - whether it should accept element that it cannot check,
   because they are outside of the viewport

Bug: b/171464034
Change-Id: Ifa7de037325a10ca4c5ed5fe45d962f5289e8c76
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2513646
Commit-Queue: Stephane Zermatten <szermatt@chromium.org>
Reviewed-by: default avatarSandro Maggi <sandromaggi@google.com>
Cr-Commit-Position: refs/heads/master@{#823982}
parent ca26c5e7
......@@ -63,11 +63,16 @@ bool operator<(const SelectorProto::Filter& a, const SelectorProto::Filter& b) {
b.css_style().pseudo_element(),
b.css_style().value());
case SelectorProto::Filter::kOnTop:
return std::make_tuple(a.on_top().scroll_into_view_if_needed(),
a.on_top().accept_element_if_not_in_view()) <
std::make_tuple(b.on_top().scroll_into_view_if_needed(),
b.on_top().accept_element_if_not_in_view());
case SelectorProto::Filter::kBoundingBox:
case SelectorProto::Filter::kEnterFrame:
case SelectorProto::Filter::kPickOne:
case SelectorProto::Filter::kLabelled:
case SelectorProto::Filter::kOnTop:
return false;
case SelectorProto::Filter::kClosest: {
......@@ -383,6 +388,12 @@ std::ostream& operator<<(std::ostream& out, const SelectorProto::Filter& f) {
case SelectorProto::Filter::kOnTop:
out << "on_top";
if (!f.on_top().scroll_into_view_if_needed()) {
out << "(no scroll)";
}
if (f.on_top().accept_element_if_not_in_view()) {
out << "(accept not in view)";
}
return out;
case SelectorProto::Filter::FILTER_NOT_SET:
......
......@@ -1065,7 +1065,7 @@ message SelectorProto {
// - this filter only detects overlays in the current frame. To detect
// overlays that cover the frame element itself, apply this filter on the
// frame element before calling enter_frame.
EmptyFilter on_top = 13;
OnTopFilter on_top = 13;
}
}
......@@ -1182,6 +1182,26 @@ message SelectorProto {
optional int32 max_pairs = 5 [default = 50];
}
// Filter out elements covered by other elements, such as overlays.
message OnTopFilter {
// If true, scroll the element into view before checking whether
// it's on top.
//
// The logic for checking whether an element is on top only works on
// elements that are positioned within the current viewport. Setting it to
// false turns off automatic scrolling to make the element visible, so the
// caller must make sure it's already the case.
optional bool scroll_into_view_if_needed = 1 [default = true];
// If true and the element cannot be scrolled into view, so the filter
// cannot check whether the element is on top, keep the element in the match
// set.
//
// This can be combined with scroll_into_view_if_needed=false to make
// this filter best effort and only check elements that are already in view.
optional bool accept_element_if_not_in_view = 2;
}
message EmptyFilter {}
reserved 1 to 8;
......
......@@ -191,8 +191,11 @@ bool ElementFinder::JsFilterBuilder::AddFilter(
AddLine(R"(elements = elements.filter((e) => {
if (e.getClientRects().length == 0) {
return false;
}
e.scrollIntoViewIfNeeded(false);
})");
if (filter.on_top().scroll_into_view_if_needed()) {
AddLine("e.scrollIntoViewIfNeeded(false);");
}
AddLine(R"(
const bounds = e.getBoundingClientRect();
const x = bounds.x + bounds.width / 2;
const y = bounds.y + bounds.height / 2;
......@@ -205,7 +208,15 @@ bool ElementFinder::JsFilterBuilder::AddFilter(
let root = document;
while (root) {
const atPoint = root.elementFromPoint(x, y);
if (!atPoint) return false;
if (!atPoint) {
)");
if (filter.on_top().accept_element_if_not_in_view()) {
AddLine("return true;");
} else {
AddLine("return false;");
}
AddLine(R"(
}
for (const target of targets) {
if (target === atPoint || target.contains(atPoint)) {
return true;
......
......@@ -2347,16 +2347,45 @@ IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, OnTop) {
}
IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, OnTopNeedsScrolling) {
TestScrollIntoView(0, 0);
Selector button({"#scroll_item_5"});
RunLaxElementCheck(button, true);
button.proto.add_filters()->mutable_on_top();
RunLaxElementCheck(button, true);
// Scroll button out of the viewport.
double target_bottom = content::EvalJs(shell(),
R"(
const target = document.getElementById("touch_area_one");
const box = target.getBoundingClientRect();
window.scrollBy(0, box.bottom + 10);
target.getBoundingClientRect().bottom
)")
.ExtractDouble();
// Before running the test, verify that the target is outside of the viewport,
// as we wanted. full_height_section guarantees that this is never a problem.
ASSERT_LE(target_bottom, 0);
Selector target({"#touch_area_one"});
RunLaxElementCheck(target, true);
auto* on_top = target.proto.add_filters()->mutable_on_top();
// Apply on_top without scrolling.
on_top->set_scroll_into_view_if_needed(false);
RunLaxElementCheck(target, false);
on_top->set_accept_element_if_not_in_view(true);
RunLaxElementCheck(target, true);
// Allow on_top to scroll.
on_top->set_scroll_into_view_if_needed(true);
on_top->set_accept_element_if_not_in_view(false);
RunLaxElementCheck(target, true);
ASSERT_GE(content::EvalJs(shell(),
R"(
document.getElementById("touch_area_one").getBoundingClientRect().bottom
)")
.ExtractDouble(),
0);
ShowOverlay();
RunLaxElementCheck(button, false);
RunLaxElementCheck(target, false);
}
IN_PROC_BROWSER_TEST_F(WebControllerBrowserTest, ALabelIsNotAnOverlay) {
......
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