Skip to content
Commit 73b4c329 authored by Charles Meng's avatar Charles Meng Committed by Chromium LUCI CQ
Browse files

[DefaultPrompt] Fix height of infobar increasing after switching tabs

Previously, the first default browser info bar shown would have a height
of 47px until you switch tabs, after which every info bar would be 56px.
The reason for this was because the info bar sets its height in
InfoBarView::RecalculateHeight [1] based on its tallest child (including
child's margin), which typically is the set as default button. However
the first time the info bar is rendered, the set as default button has
height 0, so the height of the info bar is set based on the next tallest
element. The button gets its correct height after SizeToPreferredSize
is called on it from ConfirmInfoBar::Layout [2], which happens after the
first RecalculateHeight, so only after switching tabs and back again,
does RecalculateHeight get called again, this time with the correct
button height.

This CL fixes the issue by setting a constant for the height of all info
bars, getting rid of RecalculateHeight. This height is based off the
spec, where buttons are 36dp and the vertical padding should be 8dp top
and bottom. A intended consequence of this is that the alternate nav
info bar will always have the same height as other info bars with
buttons (confirm/tab sharing).

Previously, the alternate nav info bar had the same height as the
initial height of confirm/tab sharing info bars, and after switching
tabs, confirm/tab sharing info bars would grow taller and no longer have
the same height. If this bug had been fixed by correctly setting the
buttons' heights from the beginning, and leaving RecalculateHeight as
is, the confirm/tab sharing info bars would then be taller than the
alternate nav info bar, since they have buttons and the alternate nav
info bar does not.

A related bug [3] that was found during the implementation of this CL,
is that buttons on certain platforms or with certain languages can have
a different font height, and buttons base their heights on their font's
height + padding, so they may not be exactly 36dp tall. As a result the
vertical padding of info bars may differ from 8dp, but at least they
will still have the correct total height, and the button will still be
vertically centered.

[1] https://crsrc.org/chrome/browser/ui/views/infobars/infobar_view.cc?l=136
[2] https://crsrc.org/chrome/browser/ui/views/infobars/confirm_infobar.cc?l=72
[3] https://b.corp.google.com/issues/330737852

Fixed: 326078969
Change-Id: Ic967c58cba39f2775dc676956b3faf37f06d3248
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5371978


Reviewed-by: default avatarDana Fried <dfried@chromium.org>
Commit-Queue: Charles Meng <charlesmeng@chromium.org>
Reviewed-by: default avatarAlison Gale <agale@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1277111}
parent fe430f7a
Loading
Loading
Loading
Loading
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment