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

Issue 768261 link

Starred by 6 users

Issue metadata

Status: Fixed
Owner:
Last visit > 30 days ago
Closed: Sep 2017
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

Android spell check replacement has larger font size than original word with certain markup

Project Member Reported by rlanday@chromium.org, Sep 25 2017

Issue description

Chrome Version: 63.0.3223.0
OS: Android N

What steps will reproduce the problem?
(1) adb push the attached HTML file (spell_check_wrong_size.html) to an Android device and open it in Chrome
(2) Tap on "Dsf" to focus the editable region
(3) Type some misspelled words
(4) Tap on the words until one shows at least one suggestion in the spell check menu
(5) Tap on the suggestion

What is the expected result?

The replaced word should have the same size as the original word.

What happens instead?

The new word is larger.


The markup that triggers the issue looks like this:

<div contenteditable>
  <span style="font-size: 12px;">Dsf</span>
</div>

The bug is that the replacement seems to have the default font size explicitly set on it, rather than the 12px size the surrounding (and original) text has.
 
spell_check_wrong_size.html
90 bytes View Download
Cc: pdr@chromium.org aelias@chromium.org
Components: Blink>TextAutosize
This is related to text autosizing.

In ReplaceSelectionCommand::SetUpStyle(), we slurp up the computed style from the selection start's anchor node:
https://chromium.googlesource.com/chromium/src/+/703c6b6cf1bcd212168721e72bc79b4afacb370e/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp#1023

Then in ReplaceSelectionCommand::CompleteHTMLReplacement(), we call ApplyStyle() to apply it:
https://chromium.googlesource.com/chromium/src/+/703c6b6cf1bcd212168721e72bc79b4afacb370e/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommand.cpp#1774

The problem is that text auto-sizing increases the computed size of the text (to 15.405px when I test). You can see this in the element inspector: the span with font-size: 12px set has "15.405px" as the computed size. ReplaceSelectionCommand faithfully puts "font-size: 15.405px" on the new text. Text auto-sizing then runs *again* on the new text to make its computed size even bigger than 15.405px.

TextAutosizer computes the enlarged size here:
https://chromium.googlesource.com/chromium/src/+blame/9250c66f40680b3e98437c02d3839d2f9a8cc305/third_party/WebKit/Source/core/layout/TextAutosizer.cpp#1338

pdr@, do you know how we can fix this so the round-trip of "read existing computed font-size, then wrap some text in a <span> with that set as the value of the font-size property" doesn't change the font size?
I suspect changing the design of text autosizing might be quite involved, but there's probably a relatively straightforward fix/hack we can make in ApplyStyleCommand and/or ReplaceSelectionCommand to work around this behavior.

Comment 3 by pdr@chromium.org, Sep 25 2017

Sorry for the delay, just getting back from BlinkOn/Tokyo. Can you try using SpecifiedFontSize which should be the original 12px? The autosizer will then come in behind you and boost that to 15.405px.
Project Member

Comment 4 by bugdroid1@chromium.org, Sep 27 2017

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

commit 9a75502e33aea7c30eba8ebe16eeb67acfc893d3
Author: Ryan Landay <rlanday@chromium.org>
Date: Wed Sep 27 03:10:38 2017

Fix ReplaceSelectionCommand inflating text size on Android

We currently have a problem where replacing a word in a richly editable element
using the Android spell check menu sometimes replaces the word with one of a
larger font size. This is because text autosizing (to make smaller text more
visible on mobile) increases the computed value of the font-size property. So if
we read the computed value off the existing text and try to use it on new text,
we're actually setting a value larger than the original (and, due to font
scaling, it will be increased to an even larger size).

The fix is to make EditingStyle ignore the computed value of the font-size
property and instead use the size from ComputedStyle::SpecifiedFontSize().

Bug:  768261 
Change-Id: I9f3aca178fcb6c4105029dc5339d173a888f9a47
Reviewed-on: https://chromium-review.googlesource.com/683229
Commit-Queue: Ryan Landay <rlanday@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Xiaocheng Hu <xiaochengh@chromium.org>
Cr-Commit-Position: refs/heads/master@{#504566}
[add] https://crrev.com/9a75502e33aea7c30eba8ebe16eeb67acfc893d3/third_party/WebKit/LayoutTests/editing/execCommand/insert-paragraph-into-table-expected.txt
[modify] https://crrev.com/9a75502e33aea7c30eba8ebe16eeb67acfc893d3/third_party/WebKit/Source/core/editing/EditingStyle.cpp
[modify] https://crrev.com/9a75502e33aea7c30eba8ebe16eeb67acfc893d3/third_party/WebKit/Source/core/editing/commands/ReplaceSelectionCommandTest.cpp

Labels: M-63
Status: Fixed (was: Assigned)
Issue 395079 has been merged into this issue.

Sign in to add a comment