New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 882525 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 11
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Windows , Chrome , Mac
Pri: 2
Type: Bug



Sign in to add a comment

Allow experimentation with different font weights for primary info

Project Member Reported by ftirelo@chromium.org, Sep 10

Issue description

Chrome Version: M70
OS: Desktop

Implementation bug for crbug.com/882523.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Sep 11

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

commit 3773deaeaeb80d86a102e84109239fc586ed0a82
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Tue Sep 11 15:21:52 2018

[AF] Add flag to enable custom font weight for primary info

The goal is to allow experimentation with different font weights
for primary info for Payment Methods and Addresses suggestions on
the Autofill dropdown.

Screenshots on Windows (Googlers only): https://drive.google.com/open?id=1BhZdvN0l9GTzZv2kvim7_OCE9r9OsOhK

Bug:  882525 
Change-Id: I9d1ec9ad662e858bd0d9c85c3b31849f13220b86
Reviewed-on: https://chromium-review.googlesource.com/1217171
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#590316}
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/chrome/browser/about_flags.cc
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/components/autofill/core/browser/autofill_experiments.h
[modify] https://crrev.com/3773deaeaeb80d86a102e84109239fc586ed0a82/tools/metrics/histograms/enums.xml

Status: Fixed (was: Assigned)
Project Member

Comment 3 by bugdroid1@chromium.org, Sep 13

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

commit 8a2d5a79ca48081949f81d22c280bbe061ec6407
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Thu Sep 13 15:36:29 2018

[AF] Fix medium font weight and remove unused options

There are many intricacies involved in getting medium font
weight, since this is not a common terminology across
platforms. Reusing TypographyProvider::MediumWeightForUI(),
which implements a best-effort approach for handling those
issues.

To avoid adding a dependency from //components/autofill to
//ui/views, this CL also introduces an enum
ForcedFontWeight to indicate the font weight that should
be applied. This way, TypographyProvider's uses will be
restrained to //c/b/ui/views/autofill.

Also remove flag options for "semi-bold" and "extra-bold",
which were already failed UX pre-analysis, and won't be
used anyways - so we don't need to add values to the enum
that will eventually be removed.

Note: some of the code for the dropdown layout experiment
is leaking to iOS, which was unintended. Will clean it
up on a follow-up.

Bug:  882525 
Change-Id: I4b3f371c76c7f8e6419b296a9aa70442a8079dd4
Reviewed-on: https://chromium-review.googlesource.com/1221688
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#591023}
[modify] https://crrev.com/8a2d5a79ca48081949f81d22c280bbe061ec6407/chrome/browser/about_flags.cc
[modify] https://crrev.com/8a2d5a79ca48081949f81d22c280bbe061ec6407/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc
[modify] https://crrev.com/8a2d5a79ca48081949f81d22c280bbe061ec6407/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/8a2d5a79ca48081949f81d22c280bbe061ec6407/components/autofill/core/browser/autofill_experiments.h

Labels: Merge-Request-70
Cc: gov...@chromium.org abdulsyed@chromium.org
Currently being discussed with govind@ and abdulsyed@ on a separate thread.
Labels: OS-Chrome OS-Linux OS-Mac OS-Windows
Project Member

Comment 7 by sheriffbot@chromium.org, Sep 14

Labels: -Merge-Request-70 Merge-Review-70 Hotlist-Merge-Review
This bug requires manual review: M70 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), geohsu@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: -Merge-Review-70 Merge-Approved-70
Branch:3538 for M70
Project Member

Comment 9 by bugdroid1@chromium.org, Sep 14

Labels: -merge-approved-70 merge-merged-3538
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a1058312bf383a780965134a9b6571f15fcce617

commit a1058312bf383a780965134a9b6571f15fcce617
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Fri Sep 14 20:20:09 2018

[M70 merge][AF] Add flag to enable custom font weight for primary info

The goal is to allow experimentation with different font weights
for primary info for Payment Methods and Addresses suggestions on
the Autofill dropdown.

Screenshots on Windows (Googlers only): https://drive.google.com/open?id=1BhZdvN0l9GTzZv2kvim7_OCE9r9OsOhK

Bug:  882525 
Change-Id: I9d1ec9ad662e858bd0d9c85c3b31849f13220b86
Reviewed-on: https://chromium-review.googlesource.com/1217171
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#590316}(cherry picked from commit 3773deaeaeb80d86a102e84109239fc586ed0a82)
Reviewed-on: https://chromium-review.googlesource.com/1227295
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#420}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/chrome/browser/about_flags.cc
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/chrome/browser/flag_descriptions.cc
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/chrome/browser/flag_descriptions.h
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/components/autofill/core/browser/autofill_experiments.h
[modify] https://crrev.com/a1058312bf383a780965134a9b6571f15fcce617/tools/metrics/histograms/enums.xml

Project Member

Comment 10 by bugdroid1@chromium.org, Sep 14

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

commit 4c6ed66d95669d296e09e688a1414eee098388b6
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Fri Sep 14 20:22:06 2018

[M70 merge][AF] Fix medium font weight and remove unused options

There are many intricacies involved in getting medium font
weight, since this is not a common terminology across
platforms. Reusing TypographyProvider::MediumWeightForUI(),
which implements a best-effort approach for handling those
issues.

To avoid adding a dependency from //components/autofill to
//ui/views, this CL also introduces an enum
ForcedFontWeight to indicate the font weight that should
be applied. This way, TypographyProvider's uses will be
restrained to //c/b/ui/views/autofill.

Also remove flag options for "semi-bold" and "extra-bold",
which were already failed UX pre-analysis, and won't be
used anyways - so we don't need to add values to the enum
that will eventually be removed.

Note: some of the code for the dropdown layout experiment
is leaking to iOS, which was unintended. Will clean it
up on a follow-up.

Bug:  882525 
Change-Id: I4b3f371c76c7f8e6419b296a9aa70442a8079dd4
Reviewed-on: https://chromium-review.googlesource.com/1221688
Reviewed-by: Vasilii Sukhanov <vasilii@chromium.org>
Reviewed-by: Tommy Martino <tmartino@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#591023}(cherry picked from commit 8a2d5a79ca48081949f81d22c280bbe061ec6407)
Reviewed-on: https://chromium-review.googlesource.com/1227221
Reviewed-by: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/branch-heads/3538@{#423}
Cr-Branched-From: 79f7c91a2b2a2932cd447fa6f865cb6662fa8fa6-refs/heads/master@{#587811}
[modify] https://crrev.com/4c6ed66d95669d296e09e688a1414eee098388b6/chrome/browser/about_flags.cc
[modify] https://crrev.com/4c6ed66d95669d296e09e688a1414eee098388b6/chrome/browser/ui/views/autofill/autofill_popup_view_native_views.cc
[modify] https://crrev.com/4c6ed66d95669d296e09e688a1414eee098388b6/components/autofill/core/browser/autofill_experiments.cc
[modify] https://crrev.com/4c6ed66d95669d296e09e688a1414eee098388b6/components/autofill/core/browser/autofill_experiments.h

Please notice that there were two CLs for this change. Both were merged to the branch.
Project Member

Comment 12 by bugdroid1@chromium.org, Oct 2

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

commit b2745b7aac27aec3917e922f18fdf9437d3a604f
Author: Fabio Tirelo <ftirelo@chromium.org>
Date: Tue Oct 02 23:36:06 2018

[AF] Add field trial config for AutofillPrimaryInfoStyleExperiment

Bug:  882525 
Change-Id: I4eec6fca2f24fc50a9004d3f2e2aa816b0a9951c
Reviewed-on: https://chromium-review.googlesource.com/c/1240393
Reviewed-by: Jesse Doherty <jwd@chromium.org>
Commit-Queue: Fabio Tirelo <ftirelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#596043}
[modify] https://crrev.com/b2745b7aac27aec3917e922f18fdf9437d3a604f/testing/variations/fieldtrial_testing_config.json

Sign in to add a comment