Some weeks ago I wrote a twitter thread explaining the process of fixing a Chromium bug related to wavy text decorations. At first it was just a patch in Chromium, but we also fixed the same issue in WebKit, which unveiled a mistake on the initial fix in Chromium (a win-win situation).
This blog post is somehow a story around web platform features implementation, which highlights the importance of interoperability and how investigating and fixing bugs on different web engines usually leads to gains for the whole ecosystem.
Let’s start from the beginning. Igalia (as part of our collaboration with Bloomberg) is working on adding support for
::grammar-error highlight pseudo-elements in Chromium.
My colleague Delan Azabani has been leading this effort. If you want more details about this work you can read her two blog posts. Also don’t miss the chance to enjoy her amazing talk from last BlinkOn 15, this talk gives lots of details about how highlight pseudos work, and includes some cool animations that help to understand this complex topic.
Lately I’ve been also helping with some related tasks here and there. Next I’m going to talk about one of them.
Spelling and grammar error markers
As you probably know spelling and grammar error markers use wavy underlines in some platforms like Linux or Windows, though not in all of them as they use dotted underlines on Mac. In Chromium they’re painted on a separated codepath, totally independent of how CSS text decorations are painted. You can easily spot the difference between a “native” spelling errors (left) and elements with
text-decoration: wavy red underline (right) in the next picture.
As part of our work around
::spelling|grammar-error highlight pseudos, we plan to merge both codepaths and use the CSS one for painting the default spelling and grammar error markers in the future. This doesn’t mean that they’ll look the same, actually they will still have a different rendering so the user can differentiate between a spelling marker and a wavy text decoration (like it happens now). But they’ll share the same code, so any improvement we do will apply to both of them.
There have been some bugs on each of them in the past, related to invalidation and overflow issues, and they had to be fixed in two places instead of just one. That’s why we’re looking into sharing the code, as its main job is to produce very similar things.
The issue we’re describing in the next section doesn’t happen on native spelling error markers, but as we plan to follow the CSS codepath, we had to fix it as a preliminary task getting things ready to move the spelling markers to use that codepath.
One problem with wavy text decorations in Chromium was that they sometimes don’t cover the full length of the text. This is because it only paints whole cycles and thus fall short in some situations.
A simple example is a wavy underline (
text-decoration: wavy green underline) on a “m” letter using big fonts (see the picture below and how it doesn’t cover the full length of the letter).
Fixing the problem
This section goes into some implementation details about how this works on Chromium and how we fixed it.
To draw wavy text decorations Chromium defines a vector path for a Bezier curve, that path is generated at
TextDecorationInfo::PrepareWavyStrokePath(). The comment in that method is quite self explanatory:
This method generates the path for wavy text decorations using the next loop:
As you can see, it only uses whole cycles of the wave (
2 * step), and it never splits that in smaller chunks. If we’re going to end up further away than the text size, we don’t add that wave to the path (
x + 2 * step <= x2). Which leads to the wrong behavior we saw in the example of the “m” letter above, where the text decoration falls short.
To prevent this problem, the code was using the method
AdjustStepToDecorationLength(), that was expected to adjust the length of a whole wave. If that method was working properly, we would always cover the full text width adjusting the size of the waves. However there were two different problems on that method:
- On one side, that method adjusted the
step, but we were always generating whole waves (
2 * step), so we might need to adjust the whole length of the wave instead.
- On the other side, the method had some bug, as it was changing the
stepwhen it was not actually needed. For example if you pass a total length of
40pxand a step of
10px, this method was adjusting the step to
10.75px, which makes no sense.
Digging a little bit on the repository’s history, we found out that this method has been around since 2013 thus it was present in both Blink and WebKit. As it has a bunch of issues and our proposed fix was cutting the waves at any point, we decided it was not needed to try to adjust their size anymore, so we get rid of this method.
The solution we used to the length issue requires two main changes:
- First we generate two extra waves before and after the text width:
- Then clip the path so it’s no longer than the text width. For which
GraphicsContextStateSaverwas really useful to just clip things related to the line that is currently being painted (for example in cases where you have both underline and overline text decorations).
The reviewers liked the idea and the patch landed in Chromium 97.0.4692 with some internal tests (not using WPT tests as how wavy lines are painted is not defined per spec and varies between implementations).
To finish this section, below there is a screenshot of the “m” with wavy green underline after this patch.
WebKit & WPT test
While looking into the history of
AdjustStepToDecorationLength() method we ended up looking into some old WebKit patches, we realized that the code for the wavy text decoration in WebKit is still very similar to Chromium, and that this very same issue was also present in WebKit. For that reason we decided to fix this problem also in WebKit too, with the same approach than the patch in Chromium.
The cool thing is that during the patch review Myles Maxfield suggested to create a mismatch reference test.
Just an aside quick explanation, reference tests (reftests) usually compare a screenshot of the test with a screenshot of the reference file, to see if the rendered output matches exactly or not between both files. But sometimes browsers do a different thing that is called mismatch reftests, which compares a test with a reference and checks that they’re actually different.
The idea here was to do a test that has a wavy text decoration but we hide most of the content with some element on top of that, and just show the bottom right corner of the decoration. We mismatch against a blank page, because there should be something painted there, if the wavy text decoration cover the whole line.
So we wrote WPT tests, that we can share between implementations, to check that this was working as expected. And while working on that test we discovered an issue on the initial Chromium fix, as the wavy underline was kind of misplaced to the left. More about that later.
On top of that there was another issue, WPT mismatch tests were not supported by WebKit tests importer, so we also added support for that in order to be able to use these new tests on the final WebKit patch fixing the wavy text decorations length which is included in Safari Technology Preview 136.
Again let’s finish the section with a screenshot of the “m” with wavy green underline after the WebKit patch.
As mentioned in the previous section, thanks to porting the patch to WebKit and working on a WPT test we found out a mistake on the first Chromium fix.
So we’re back in Chromium where we were clipping the wavy text decoration with the wrong offset, so it looks like it was a little bit shifted to the left (specially when using big fonts). I’m repeating here the image for the initial Chromium fix, adding a grey background so it’s easier to notice the problem and compare with the final fix. There you can see that the wavy underline starts more on the left than expected, and ends earlier than the “m” letter.
The patch to fix that was pretty simple, so we landed it in Chromium 98.0.4697. And this is the final output with the text decoration positioned in the proper place.
In addition to an improved rendering on static wavy text decorations, these fixes have some nice effects when animations are involved. See the following video showing an example with animations (stealing a
letter-spacing example from Delan’s latest blog post), on the left you can see Chromium (buggy version on top, fixed one on bottom) and on the right WebKit (again buggy on top and fixed on bottom).
But as usual there’s still something else, in this case very related to this topic. There’s the concept of decorating box in CSS Text Decoration spec, and that hasn’t been implemented in Chromium and WebKit yet (though Firefox seems to be doing that right in most cases, except for dotted text decorations).
This issue is quite noticeable when you have different elements in the same line (like a
<strong> element), or different font sizes in the same line. See the next video that shows this problem in Chromium (on the left) and Firefox (on the right, where only dotted text decorations have problems).
This has been a problem in Chromium and WebKit forever, even native spelling and grammar error markers have the same issue (thought it’s less noticeable as they’re painted always in a small size). Though even when this isn’t a strictly a blocker for all this work, this is something we’re looking forward to get fixed too.
The main takeaway from this blog post is how browser interoperability plays a key important role in the implementation of web platform features. Which is something we’re very concerned of at Igalia.
The fact that we fixed this issue in Chromium and WebKit at the same time helped to get more eyes looking into the same code, which is usually very beneficial.
We ended up not just fixing the issue in both implementations (Chromium and WebKit), but also adding new WPT tests that would be useful for any other implementation and to prevent regressions in the future. Even as a positive side effect, WebKit added support for mismatch reference tests as part of this work.
Finally, thanks to all the people that helped to make this happen by providing ideas, feedback and reviewing the patches; particularly Delan Azabani (Igalia), Myles Maxfield (Apple) and Stephen Chenney (Google).