Skip to content

Commit

Permalink
[m115][omnibox][cr23] Fix cursor position test for SS height.
Browse files Browse the repository at this point in the history
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 a7d95bf)

Bug: 1416351, 1453711, 1368852
Change-Id: I78249edfd48ddafbce28c46aed2abcbe65461ac8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4606073
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Code-Coverage: Findit <findit-for-me@appspot.gserviceaccount.com>
Reviewed-by: Katie Dektar <katie@chromium.org>
Auto-Submit: manuk hovanesian <manukh@chromium.org>
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: 1d71a33-refs/heads/main@{#1148114}
  • Loading branch information
manukh authored and Chromium LUCI CQ committed Jun 13, 2023
1 parent 3b904ae commit b691d36
Showing 1 changed file with 13 additions and 4 deletions.
Expand Up @@ -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());
Expand Down

0 comments on commit b691d36

Please sign in to comment.