New issue
Advanced search Search tips

Issue 903554 link

Starred by 1 user

Issue metadata

Status: Verified
Owner:
Closed: Nov 15
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug



Sign in to add a comment

[AF Android] Multiline rows are clipped

Project Member Reported by tmartino@chromium.org, Nov 8

Issue description

The height on multiline rows is too small, so text on these items gets cut off.

A snippet responsible for this edge case did not get carried over when refactoring. Easy fix to reintroduce it.
 
Screenshot attached since my original report is not super clear.

Also want to add, as far as I've seen, this only affects older (pre-Nougat) devices.
clipped-error.png
43.1 KB View Download
Project Member

Comment 2 by bugdroid1@chromium.org, Nov 13

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/9196ef69e8a4d6ed2f26caf2c9ba0d8f01f07abe

commit 9196ef69e8a4d6ed2f26caf2c9ba0d8f01f07abe
Author: Tommy Martino <tmartino@chromium.org>
Date: Tue Nov 13 20:31:06 2018

[AF Android] Fix height of multiline rows

Multiline entries are used mostly for warnings and other edge cases
where descriptive text is needed. By their nature the height is
variable, so we need to set padding and use a content-wrapping height.
This was done in the old implementation but errantly not carried over to
the refactor.

Bug:  903554 
Change-Id: I6e9873c757fb9087af3066bb55017badccd15149
Reviewed-on: https://chromium-review.googlesource.com/c/1327579
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Commit-Position: refs/heads/master@{#607724}
[modify] https://crrev.com/9196ef69e8a4d6ed2f26caf2c9ba0d8f01f07abe/components/autofill/android/java/res/layout/autofill_dropdown_item_refresh.xml

Labels: Merge-Request-71
Status: Verified (was: Started)
Verified in Canary.

Requesting merge to M71. I'm aware that this is quite late in the cycle, but I  think the benefit greatly outweighs the risk in this case. The bug in question is a user-facing visual regression that makes important security text unreadable. The fix is incredibly simple--just 2 lines of XML are changed--and not only fixes this particular bug, but generally makes the UI more resilient to this class of bug.
Project Member

Comment 4 by sheriffbot@chromium.org, Nov 15

Labels: -Merge-Request-71 Hotlist-Merge-Review Merge-Review-71
This bug requires manual review: M71 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: benmason@(Android), kariahda@(iOS), kbleicher@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Hotlist-Merge-Review -Merge-Review-71 Merge-Approved-71
Approved for merge to 71, branch 3578.
Labels: -Merge-Approved-71 Merge-Merged-71-3578
The following revision refers to this bug: 
https://chromium.googlesource.com/chromium/src.git/+/090f810a80a320333d338f723406ce78608c27b0

Commit: 090f810a80a320333d338f723406ce78608c27b0
Author: tmartino@chromium.org
Commiter: ftirelo@chromium.org
Date: 2018-11-20 22:19:46 +0000 UTC

[Merge M71][AF Android] Fix height of multiline rows

Multiline entries are used mostly for warnings and other edge cases
where descriptive text is needed. By their nature the height is
variable, so we need to set padding and use a content-wrapping height.
This was done in the old implementation but errantly not carried over to
the refactor.

Bug:  903554 
Change-Id: I6e9873c757fb9087af3066bb55017badccd15149
Reviewed-on: https://chromium-review.googlesource.com/c/1327579
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607724}(cherry picked from commit 9196ef69e8a4d6ed2f26caf2c9ba0d8f01f07abe)
Reviewed-on: https://chromium-review.googlesource.com/c/1344879
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#778}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
Project Member

Comment 7 by bugdroid1@chromium.org, Nov 20

Labels: merge-merged-3578
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/090f810a80a320333d338f723406ce78608c27b0

commit 090f810a80a320333d338f723406ce78608c27b0
Author: Tommy Martino <tmartino@chromium.org>
Date: Tue Nov 20 22:19:46 2018

[Merge M71][AF Android] Fix height of multiline rows

Multiline entries are used mostly for warnings and other edge cases
where descriptive text is needed. By their nature the height is
variable, so we need to set padding and use a content-wrapping height.
This was done in the old implementation but errantly not carried over to
the refactor.

Bug:  903554 
Change-Id: I6e9873c757fb9087af3066bb55017badccd15149
Reviewed-on: https://chromium-review.googlesource.com/c/1327579
Reviewed-by: Theresa <twellington@chromium.org>
Commit-Queue: Tommy Martino <tmartino@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#607724}(cherry picked from commit 9196ef69e8a4d6ed2f26caf2c9ba0d8f01f07abe)
Reviewed-on: https://chromium-review.googlesource.com/c/1344879
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3578@{#778}
Cr-Branched-From: 4226ddf99103e493d7afb23a4c7902ee496108b6-refs/heads/master@{#599034}
[modify] https://crrev.com/090f810a80a320333d338f723406ce78608c27b0/components/autofill/android/java/res/layout/autofill_dropdown_item_refresh.xml

This issue is fixed in current M71 build 71.0.3578.75

Sign in to add a comment