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

Issue 666378 link

Starred by 3 users

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2016
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Regression
Team-Accessibility

Blocked on:
issue 645717



Sign in to add a comment

accessibility settings on amazon.com

Reported by huape...@amazon.com, Nov 17 2016

Issue description

Chrome Version       : latest master, v54
URLs (if applicable) : www.amazon.com

What steps will reproduce the problem?
(1) go to www.amazon.com
(2) go to Setting -> Accessibility -> Text Scaling, and change scaling from 100% to 200%
(3) There is a blank space on the right side of the page.

What is the expected result?
fonts get bigger

What happens instead?
fonts get bigger, but there is a blank space on the right side of the page

Please provide any additional information below. Attach a screenshot if
possible.
I tested it on nexus 7. I started noticing this since v54. I can't repro this on v53.
 
Cc: candr...@chromium.org
Components: UI>Accessibility
Labels: -Type-Bug -Pri-3 M-54 OS-Android Pri-1 Type-Bug-Regression
As per the details in the bug, this looks like an android issue. Tagging accordingly.

Comment 2 Deleted

Comment 3 by kra...@amazon.com, Nov 17 2016

This issue is not specific to amazon.com
gmail.com (post login) also has an absolutely broken UI in v54+ (to the point of being unusable) when using 200% zoom and it looks perfectly fine in v53 and earlier

(Sorry for the delete above - the comment was unclear in regards to this only happening with active zoom. Gmail of course looks fine when no zoom setting is set)
Cc: krav...@chromium.org
Labels: triage-te
kravula@ - can you try to repro?
Cc: -candr...@chromium.org -krav...@chromium.org aelias@chromium.org
Labels: -triage-te
Owner: pdr@chromium.org
Status: Assigned (was: Unconfirmed)
Tested with Nexus7/LMY47T and able to repro on M54 build

Bisect info
Good Build-54.0.2840.33
Bad Build -54.0.2840.34
CL range -https://chromium.googlesource.com/chromium/src/+log/54.0.2840.33..54.0.2840.34?pretty=fuller&n=10000

Suspect CL -https://chromium.googlesource.com/chromium/src/+/d25ac697753f02ac5c923d65402d75b4211cab28
Review-Url: https://codereview.chromium.org/2340553003
Labels: hasbisect-per-revision

Comment 8 by kra...@amazon.com, Nov 18 2016

I just reverted d25ac697753f02ac5c923d65402d75b4211cab28 in a v54 workspace and it DOES fix the issue.
Great find kravula@! :)

Comment 9 by aelias@chromium.org, Nov 18 2016

Blockedon: 645717

Comment 10 by kra...@amazon.com, Nov 18 2016

Please note that this isn't just specific to Chromium's font size setting. (Which used to be separate from Android platform font size)
Changing Android's font size setting will now also cause this issue.
--> This might potentially impact more users than initially assumed.

Comment 11 by pdr@chromium.org, Nov 18 2016

This will indeed be fixed by 645717.

I am still a little puzzled why the change to honor text-size-adjust would cause this breakage; we should have honored the accessibility font scale factor before and after the change. I will investigate this independently of Aelias's work in 645717.

Comment 12 by kra...@amazon.com, Nov 18 2016

Great, thanks pdr! :)
Do you by any chance know if 645717 is planned to go into v55?

Comment 13 by pdr@chromium.org, Nov 18 2016

645717 is not currently planned for v55.

Comment 14 by kra...@amazon.com, Nov 19 2016

Good to know, thanks!

Is there any chance this might get re-prioritized? (Or worst case putting a revert of d25ac697 into v55 as a stopgap?)
I don't know the exact numbers of Android users that have a large font size, but even if we are only talking 1%, that would be tens of millions of users who can't use Gmail, Google Flights, or thousands of other websites. :(

Comment 15 Deleted

Comment 16 by pdr@chromium.org, Nov 19 2016

This is indeed a bug. Patch up to fix it: https://codereview.chromium.org/2514733005
Project Member

Comment 17 by bugdroid1@chromium.org, Nov 22 2016

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

commit 394de7b0baa6b0136b6acfd15486de6a98e19152
Author: pdr <pdr@chromium.org>
Date: Tue Nov 22 02:54:40 2016

Only apply accessibility font scale factor when autosizing inflates

The accessibility font scale factor is currently applied to all text
that the text autosizer would inflate (this is changing in
crbug.com/645717). crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b
introduced a bug where text that would not be inflated would become
inflated if text-size-adjust: none was applied.

This patch only applies the accessibility font scale factor if the
text autosizer would have inflated the text already. A new test,
AccessibilityFontScaleFactorWithTextSizeAdjustNone, has been added.
Other tests in TestAutosizerTest.cpp have been reformatted to 80cols.

BUG= 666378 

Review-Url: https://codereview.chromium.org/2514733005
Cr-Commit-Position: refs/heads/master@{#433757}

[modify] https://crrev.com/394de7b0baa6b0136b6acfd15486de6a98e19152/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/394de7b0baa6b0136b6acfd15486de6a98e19152/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Comment 18 by pdr@chromium.org, Nov 22 2016

Components: Blink>TextAutosize
Labels: Merge-Request-55
TPMs, this regressed in M54 and affects users with accessibility font settings when visiting some sites that have text-size-adjust: none, including Amazon.com. I'm requesting a merge because it affects at least one large site and the fix is straightforward (3 lines).

Comment 19 by dimu@chromium.org, Nov 22 2016

Labels: -Merge-Request-55 Merge-Approved-55 Hotlist-Merge-Approved
Your change meets the bar and is auto-approved for M55 (branch: 2883)
Labels: -Merge-Merged -Merge-Approved-55 merge-merged-2883
Issue is fixed on Latest M55 Beta Build, Issue is reproducible on M56 Dev. Do we need to merge required for M56.
Status: Assigned (was: Fixed)
Can we get this merged in M56 too?
Labels: M-56

Comment 25 by pdr@chromium.org, Nov 30 2016

Labels: Merge-Request-56

Comment 26 by pdr@chromium.org, Nov 30 2016

@TPMs, this has already been merged into M55. It was only an oversight that we didn't get M56 too.

Comment 27 by dimu@chromium.org, Nov 30 2016

Labels: -Merge-Request-56 Merge-Approved-56
Your change meets the bar and is auto-approved for M56 (branch: 2924)
Project Member

Comment 28 by bugdroid1@chromium.org, Nov 30 2016

Labels: -merge-approved-56 merge-merged-2924
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e185926ca8f2014314df6b7a8c345607323d4580

commit e185926ca8f2014314df6b7a8c345607323d4580
Author: pdr@chromium.org <pdr@chromium.org>
Date: Wed Nov 30 21:10:53 2016

Only apply accessibility font scale factor when autosizing inflates

The accessibility font scale factor is currently applied to all text
that the text autosizer would inflate (this is changing in
crbug.com/645717). crrev.com/1229b1414d64c314666d2fb3bf47706f2b5e947b
introduced a bug where text that would not be inflated would become
inflated if text-size-adjust: none was applied.

This patch only applies the accessibility font scale factor if the
text autosizer would have inflated the text already. A new test,
AccessibilityFontScaleFactorWithTextSizeAdjustNone, has been added.
Other tests in TestAutosizerTest.cpp have been reformatted to 80cols.

BUG= 666378 

Review-Url: https://codereview.chromium.org/2514733005
Cr-Commit-Position: refs/heads/master@{#433757}
(cherry picked from commit 394de7b0baa6b0136b6acfd15486de6a98e19152)

Review URL: https://codereview.chromium.org/2544633002 .

Cr-Commit-Position: refs/branch-heads/2924@{#208}
Cr-Branched-From: 3a87aecc31cd1ffe751dd72c04e5a96a1fc8108a-refs/heads/master@{#433059}

[modify] https://crrev.com/e185926ca8f2014314df6b7a8c345607323d4580/third_party/WebKit/Source/core/layout/TextAutosizer.cpp
[modify] https://crrev.com/e185926ca8f2014314df6b7a8c345607323d4580/third_party/WebKit/Source/core/layout/TextAutosizerTest.cpp

Comment 29 by pdr@chromium.org, Nov 30 2016

Status: Fixed (was: Assigned)
Verified in 55.0.2883.71 with Nexus7

Sign in to add a comment