Another feedback re: text scaling on tablet
https://screenshot.googleplex.com/gf2s1iCaE1K

Bisect info

Good build -53.0.2783.5
Bad build - 53.0.2784.0

Cl Range -https://chromium.googlesource.com/chromium/src/+log/53.0.2783.5..53.0.2784.0?pretty=fuller&n=10000

Hey lpalmaro@, dmazzoni@,

I'm debating on whether or not we need to fix this with a respin or not (this bug was first found in our initial M53 stable push - it existed on beta for more than 6 months with no one noticing).  What are your thoughts on it's impact?

This is indeed still broken on trunk.  I took a look through the logs and these seems potentially vaguely familiar:

https://chromium.googlesource.com/chromium/src/+/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea
https://chromium.googlesource.com/chromium/src/+/11f4e35b660f4f5cbaed65a0a7f181b8426e877e
https://chromium.googlesource.com/chromium/src/+/1823f9957dd146fd14cec6ec8c549364fbc4fbfd

Assigning to pdr@ since the first patch mentioned text resizing.

Escalating the priority as this is currently blocking 53 stable

Do we have any examples of sites?

I have a guess at what may be happening... the accessibility feature uses the text autosizing and in m53 we started honoring "text-size-adjust: none" which disables autosizing. I suspect this broke users who were increasing the font size through the accessibility settings.

This is fixable but not in a single day. How should we proceed?

tedchoc@ and I were able to repro this on every single site, so that shouldn't be hard.

Was the change to begin honoring text-size-adjust: none put behind a Finch flag?  Can we disable it or revert it until we can fix our accessibility feature?

Makes sense, but let's confirm pdr@'s theory first.  tedchoc@ and kravula@, what URLs did you observe accessibility scale is broken on?

I can repro on gawker.com, tedchoc@ could repro on reddit.

The feature is sadly not behind a finch flag but I think we could disable it with a hack. I will prototype something right now.

we are able to repro on wikipedia.org page

screenshots @ http://go/chrome-androidlogs1/6/645269

I have a patch up that can be merged into all channels: https://codereview.chromium.org/2330513002

In parallel, Skobes is confirming whether our theory is correct.

I have confirmed that "text-size-adjust: none" disables all autosizing, causing the accessibility slider to have no effect.

I'm now building a real android build to confirm this works. Assuming it does, I'll land the above patch and request merges into all channels.

ETA ~ 2hrs?

Confirmed that https://codereview.chromium.org/2330513002 restores functionality of the accessibility slider on m.reddit.com.

Incidentally "text-size-adjust: 100%" had the same effect as "text-size-adjust: none", which may explain why so many sites were affected.

It looks like any value other than "auto" would override the slider.  Instead they probably should have been multiplied together.

Thanks a ton for such a quick fix, and I'm sorry that feedback came so late in the cycle.  Pre-approving merge for M53 branch 2785, please try to get this merged by EOD so that we can begin qualifying a new release over the weekend.

May I merge into M54 as well? I'd like to keep all the channels on the same code if possible.

Absolutely, approved for branch 2840 as well.

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

commit 4e8989e66c9cb0e427ea886dfc5ef51ef0e4e836
Author: pdr <pdr@chromium.org>
Date: Fri Sep 09 22:38:41 2016

Temporarily stop honoring text-size-adjust as it breaks accessibility

This patch temporarily diables text-size-adjust from affecting the
text autosizer because the accessibility font size adjustment also
uses the text autosizer and sites should not be able to disable the
accessibility settings.

This is a temporary change. A proper fix will be coming in a followup.

Text-size-adjust support was added in:
https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea

BUG= 645269 

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

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

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

commit 4f6673ea1b3a85a62761bb95f5808659a6f5f28f
Author: pdr@chromium.org <pdr@chromium.org>
Date: Fri Sep 09 22:49:54 2016

Temporarily stop honoring text-size-adjust as it breaks accessibility

This patch temporarily diables text-size-adjust from affecting the
text autosizer because the accessibility font size adjustment also
uses the text autosizer and sites should not be able to disable the
accessibility settings.

This is a temporary change. A proper fix will be coming in a followup.

Text-size-adjust support was added in:
https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea

BUG= 645269 

Review-Url: https://codereview.chromium.org/2330513002
Cr-Commit-Position: refs/heads/master@{#417738}
(cherry picked from commit 4e8989e66c9cb0e427ea886dfc5ef51ef0e4e836)

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

Cr-Commit-Position: refs/branch-heads/2840@{#287}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

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

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

commit ce22ac06d267ff984b41bbf90480f77bb8b20d1f
Author: pdr@chromium.org <pdr@chromium.org>
Date: Fri Sep 09 23:13:03 2016

Temporarily stop honoring text-size-adjust as it breaks accessibility

This patch temporarily diables text-size-adjust from affecting the
text autosizer because the accessibility font size adjustment also
uses the text autosizer and sites should not be able to disable the
accessibility settings.

This is a temporary change. A proper fix will be coming in a followup.

Text-size-adjust support was added in:
https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea

BUG= 645269 

Review-Url: https://codereview.chromium.org/2330513002
Cr-Commit-Position: refs/heads/master@{#417738}
(cherry picked from commit 4e8989e66c9cb0e427ea886dfc5ef51ef0e4e836)

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

Cr-Commit-Position: refs/branch-heads/2785@{#870}
Cr-Branched-From: 68623971be0cfc492a2cb0427d7f478e7b214c24-refs/heads/master@{#403382}

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

Alrighty, we should be good to go. It's always mildly terrifying to have code go from my hands straight to lots of users. I'll be online--if you hear anything please send me a message.

I'll fix the actual bug next week.

I think we're going to ship a beta update next week to soak test this and a few other things, so don't worry, it won't go straight to 100M people, just 1M :)

FWIW, I think this change is pretty low risk. I'm just being a paranoid android.

Accessibility text scaling Works as per expected behavior.
Issue Verified in Samsung Galaxy S4 (GT-I9505)/ LRX22C, Lenovo K3 Note(K50a40) / LRX21M, Nexus 6 / NRD91E on 53.0.2785.112

Issue is also verified on M54: 54.0.2840.23

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

commit eccac1fd6db85731fb95b11915fc2197a6ceb0c3
Author: pdr <pdr@chromium.org>
Date: Tue Sep 13 05:13:49 2016

Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

BUG= 645269 

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

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

The above patch is a real fix and re-enables the text-size-adjust feature. Requesting a merge so we have the text-size-adjust feature in M54 (but not M53 due to this late regression).

Your change meets the bar and is auto-approved for M54 (branch: 2840)

Adjusting the priority because the P0 has been resolved and we just need to re-enable the feature with the accessibility bug fixed. This last-minute bug caused text-size-adjust to miss M53 but I think we can get it back in M54.

Your change meets the bar and is auto-approved for M54 (branch: 2840)

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit 3ec856cdfb31f023a832547f4f702858e9b8a903
Author: aberent <aberent@chromium.org>
Date: Tue Sep 13 17:05:51 2016

Revert of Do not let text-size-adjust override accessibility font settings (patchset #2 id:20001 of https://codereview.chromium.org/2329733002/ )

Reason for revert:
Broke ContextualSearchManagerTest#testChainedSearchCreatesNewContent and ContextualSearchManagerTest#testChainedTapsRemovedFromHistory

BUG= 646342 

Original issue's description:
> Do not let text-size-adjust override accessibility font settings
>
> The text autosizer is used for both making desktop pages more legible
> on mobile, and for applying the accessibility font scale factor. When
> text-size-adjust support was added, pages were able to override the
> accessibility setting in addition to the autosizing multiplier.
>
> This patch ensures accessibility font scale settings are respected even
> with text-size-adjust. A followup bug has been filed (crbug.com/645717)
> for moving the accessibility font scale factor out of the autosizer.
>
> This patch also re-enables support for text-size-adjust which was
> disabled temporarily due to breaking accessibility settings.
>
> BUG= 645269 
>
> Committed: https://crrev.com/eccac1fd6db85731fb95b11915fc2197a6ceb0c3
> Cr-Commit-Position: refs/heads/master@{#418173}

TBR=skobes@chromium.org,pdr@chromium.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG= 645269 

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

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

I have a guess as to what broke these tests.  They test the "chained search" aspect of Contextual Search (CS) in which a tap very close to a previous tap should just update the CS Bar instead of hiding the Bar.  Our test page has two tap targets that are normally close enough to trigger this behavior, but maybe with te recent text-size-adjust change they are not close enough anymore.  Is there a way to set up these tests to have a consistent font size so this won't happen?
ContextualSearchManagerTest#testChainedSearchCreatesNewContent and ContextualSearchManagerTest#testChainedTapsRemovedFromHistory

I suspect the bisect was wrong, but I didn't get a chance to prove it yet.

The ContextualSearchManagerTest tests use android/contextualsearch/tap_test.html which has a meta viewport which should disable all text size adjustment, so I think your test is fine. Text-size-adjust has been in the tree for a few months so it shouldn't affect your tests--we just had to disable the feature temporarily due to a release blocker.

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

commit 1229b1414d64c314666d2fb3bf47706f2b5e947b
Author: pdr <pdr@chromium.org>
Date: Wed Sep 14 04:40:53 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

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

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

Closing this as fixed, will request a merge of the fix into M54 in  https://crbug.com/623158 .

This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

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

commit d25ac697753f02ac5c923d65402d75b4211cab28
Author: pdr@chromium.org <pdr@chromium.org>
Date: Tue Sep 20 17:59:25 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}
(cherry picked from commit 1229b1414d64c314666d2fb3bf47706f2b5e947b)

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

Cr-Commit-Position: refs/branch-heads/2840@{#444}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

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

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

commit 4f6673ea1b3a85a62761bb95f5808659a6f5f28f
Author: pdr@chromium.org <pdr@chromium.org>
Date: Fri Sep 09 22:49:54 2016

Temporarily stop honoring text-size-adjust as it breaks accessibility

This patch temporarily diables text-size-adjust from affecting the
text autosizer because the accessibility font size adjustment also
uses the text autosizer and sites should not be able to disable the
accessibility settings.

This is a temporary change. A proper fix will be coming in a followup.

Text-size-adjust support was added in:
https://crrev.com/5365b8ad85c2f5cee6034b8baea3c764ed78e1ea

BUG= 645269 

Review-Url: https://codereview.chromium.org/2330513002
Cr-Commit-Position: refs/heads/master@{#417738}
(cherry picked from commit 4e8989e66c9cb0e427ea886dfc5ef51ef0e4e836)

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

Cr-Commit-Position: refs/branch-heads/2840@{#287}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

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

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

commit d25ac697753f02ac5c923d65402d75b4211cab28
Author: pdr@chromium.org <pdr@chromium.org>
Date: Tue Sep 20 17:59:25 2016

(reland) Do not let text-size-adjust override accessibility font settings

The text autosizer is used for both making desktop pages more legible
on mobile, and for applying the accessibility font scale factor. When
text-size-adjust support was added, pages were able to override the
accessibility setting in addition to the autosizing multiplier.

This patch ensures accessibility font scale settings are respected even
with text-size-adjust. A followup bug has been filed (crbug.com/645717)
for moving the accessibility font scale factor out of the autosizer.

This patch also re-enables support for text-size-adjust which was
disabled temporarily due to breaking accessibility settings.

This was reverted due to a forgotten "!" in checking whether the
viewport was specified by the author in TextAutosizer.cpp. A new test
has been added to catch this in the future. The unneeded meta viewport
declarations in all other tests have been removed because they had
no effect with the meta viewport setting disabled.

Original review:
https://codereview.chromium.org/2329733002

BUG= 645269 , 646342 

Review-Url: https://codereview.chromium.org/2340553003
Cr-Commit-Position: refs/heads/master@{#418488}
(cherry picked from commit 1229b1414d64c314666d2fb3bf47706f2b5e947b)

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

Cr-Commit-Position: refs/branch-heads/2840@{#444}
Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607}

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