Issue metadata
Sign in to add a comment
|
accessibility settings on amazon.com
Reported by
huape...@amazon.com,
Nov 17 2016
|
||||||||||||||||||||||||
Issue descriptionChrome 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.
,
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)
,
Nov 18 2016
kravula@ - can you try to repro?
,
Nov 18 2016
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
,
Nov 18 2016
Similar issue -https://bugs.chromium.org/p/chromium/issues/detail?id=648928
,
Nov 18 2016
,
Nov 18 2016
I just reverted d25ac697753f02ac5c923d65402d75b4211cab28 in a v54 workspace and it DOES fix the issue. Great find kravula@! :)
,
Nov 18 2016
,
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.
,
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.
,
Nov 18 2016
Great, thanks pdr! :) Do you by any chance know if 645717 is planned to go into v55?
,
Nov 18 2016
645717 is not currently planned for v55.
,
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. :(
,
Nov 19 2016
This is indeed a bug. Patch up to fix it: https://codereview.chromium.org/2514733005
,
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
,
Nov 22 2016
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).
,
Nov 22 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Nov 22 2016
Merged in https://codereview.chromium.org/2527483002 / https://chromium.googlesource.com/chromium/src/+/5a3e6f1238c826cd4dd99c541d359471ffe181e8
,
Nov 30 2016
,
Nov 30 2016
Issue is fixed on Latest M55 Beta Build, Issue is reproducible on M56 Dev. Do we need to merge required for M56.
,
Nov 30 2016
Can we get this merged in M56 too?
,
Nov 30 2016
,
Nov 30 2016
,
Nov 30 2016
@TPMs, this has already been merged into M55. It was only an oversight that we didn't get M56 too.
,
Nov 30 2016
Your change meets the bar and is auto-approved for M56 (branch: 2924)
,
Nov 30 2016
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
,
Nov 30 2016
,
Nov 30 2016
Verified in 55.0.2883.71 with Nexus7 |
|||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||
Comment 1 by ligim...@chromium.org
, Nov 17 2016Components: UI>Accessibility
Labels: -Type-Bug -Pri-3 M-54 OS-Android Pri-1 Type-Bug-Regression