From b691d36e5f6a3db6c4d8b43ebb8b68ea42354721 Mon Sep 17 00:00:00 2001 From: manukh Date: Tue, 13 Jun 2023 20:42:14 +0000 Subject: [PATCH] [m115][omnibox][cr23] Fix cursor position test for SS height. m115 merge. https://screenshot.googleplex.com/Bz5jWYp5HkhADMB crrev.com/c/4581107 launched omnibox SS height, increasing the omnibox height from 28px to 34px. chromeOS had some rounding issues, where it actually used a 29px omnibox instead of a 28px omnibox. It also uses a 17px cursor height instead of 16px like linux. There's a test `AccessibilityHighlightsBrowserTest.CaretHighlightOmnibox` that checks the cursor is vertically centered in the omnibox. Due to the rounding issues on chromeOS, it ended up being 1px off after increasing the omnibox height to 34px. So that CL also updated the test to expect the cursor to be centered at 1px less than the center of the omnibox. But this broke the test on linux, since linux didn't have the rounding issue that chromeOS did. So this CL updates the test to expect the cursor to be EITHER centered OR 1px off from center. (cherry picked from commit a7d95bfa45bd3dcb1a3c0290b34d5d9e12686778) Bug: 1416351, 1453711, 1368852 Change-Id: I78249edfd48ddafbce28c46aed2abcbe65461ac8 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4606073 Commit-Queue: manuk hovanesian Code-Coverage: Findit Reviewed-by: Katie Dektar Auto-Submit: manuk hovanesian Cr-Original-Commit-Position: refs/heads/main@{#1156426} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4609204 Cr-Commit-Position: refs/branch-heads/5790@{#713} Cr-Branched-From: 1d71a337b1f6e707a13ae074dca1e2c34905eb9f-refs/heads/main@{#1148114} --- .../accessibility_highlights_browsertest.cc | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/chrome/browser/ash/accessibility/accessibility_highlights_browsertest.cc b/chrome/browser/ash/accessibility/accessibility_highlights_browsertest.cc index f866f6d2fd7d9..c07f24f0d7387 100644 --- a/chrome/browser/ash/accessibility/accessibility_highlights_browsertest.cc +++ b/chrome/browser/ash/accessibility/accessibility_highlights_browsertest.cc @@ -185,10 +185,19 @@ IN_PROC_BROWSER_TEST_F(AccessibilityHighlightsBrowserTest, ->GetViewByID(VIEW_ID_OMNIBOX) ->GetBoundsInScreen(); - // -1 presumably because of rounding. Visually, it looks fine (8px between the - // top of the cursor to the top of the omnibox; 7px at the bottom). Moving the - // cursor down 1px would only make it less balanced (9 & 6px). - EXPECT_EQ(bounds.CenterPoint().y(), omnibox_bounds.CenterPoint().y() - 1); + // TODO(crbug.com/1453711) Investigate why chromeOS is miscalculating the + // focus ring position in tests only. Visually, when running chromium, it + // looks right; the focus ring is centered in the omnibox. But when running + // tests, the focus ring is 1px higher than what it should be according to + // the code and visually. + int expected_offset = +#if BUILDFLAG(IS_CHROMEOS) + 1; +#else + 0; +#endif + EXPECT_TRUE(bounds.CenterPoint().y() == + omnibox_bounds.CenterPoint().y() - expected_offset); // On the left edge of the omnibox. EXPECT_LT(bounds.x(), omnibox_bounds.x());