Bug 102504 – Text Autosizing: Skip blocks with background images

• Component: Layout and Rendering
• Status: RESOLVED WONTFIX

[John Mellor]

When applying Text Autosizing to blocks of text with a fixed-height background image, it's common for the text to wrap and exceed the height of the background image.

For example on above Flickr url, they have made custom numbered list markers like:

    <style>
    li > .num {
        color: #fff;
        /* tour_blue-circle.png is an 18x18 blue bullet */
        background: url(http://l.yimg.com/g/images/tour/tour_blue-circle.png) no-repeat;
    }
    </style>
    <li><span class="num">2</span> (text goes here...) </li>

This is perfectly reasonable, but Text Autosizing ends up making the number spill out of the blue circle, and since the number and page background are both white this means it disappears!

We need to fix this category of bugs in one of two ways:
1. Skip blocks with fixed-height background images, when deciding what to autosize.
2. Scale fixed-height background images up in proportion to the text size multiplier (or even better, take wrapping into account to work out the exact height gain due to Text Autosizing).

As a short/medium-term fix, I recommend going with option 1. Once that's in place we can investigate option 2 (with lower priority).

For option 1, the main thing we need to decide is what should count as a fixed height background image: for example a block with its vertical background-size set to "contain", "cover" or a percentage value should not count as fixed. It's less clear whether a block with its vertical background-repeat set to "repeat" should count as flexible height (especially since the default value is "repeat", and so if designers don't expect the contents to exceed the size of the background image they will often leave it as its default value despite not wanting the background to repeat).

As a starting point style->hasBackgroundImage() can tell us which blocks have background images :)

[John Mellor]

Created attachment 174678 [details]
Screenshot of flickr.com list markers

[John Mellor]

Hmm, and I've been talking about blocks with fixed-height background images, but this particular example is of course an inline span with a fixed-height background image - so I guess it's not enough to do this check for blocks only. Though disabling Text Autosizing for part but not all of a line is really bad. So perhaps the scaling up approach is more practical...

Created attachment 180529 [details]
Patch

[Kenneth Rohde Christiansen]

Comment on attachment 180529 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=180529&action=review

> Source/WebCore/ChangeLog:9
> +        Prevent autosizing of containers containing elements with background images.
> +

It is always good to write the reasoning behind such a change

Created attachment 180897 [details]
Patch

uploaded a new patch with a more detailed description as suggested by Kenneth.
(looks like lost the review+ tag in the process)

[John Mellor]

Code-wise this seems fine (possibly a slight shame to introduce a separate tree traveral rather than reusing the one from containerContainsOneOfTags, but the run time will still be O(n), and I guess it's arguably clearer).

However Tim and I looked at the effect this has on websites and, while it improves the rendering of a lot of sites, it also worsens the rendering of a lot of other sites. It's quite common to include background-images inline (e.g. on a span's ::before or ::after pseudo-element), and in these cases we were no longer autosizing whole paragraphs, that did in fact require autosizing.

Tim's going to try tweaking this so it only skips blocks containing elements with background images if those elements also contain text. De-prioritizing this slightly though, as it's not as urgent as some other Text Autosizing issues.

[Anders Carlsson]

Comment on attachment 180897 [details]
Patch

Clearing review flag on patches from before 2014. If this patch is still relevant, please reset the r? flag.

[Daniel Bates]

Marking this bug Resolved WontFix because the TEXT_AUTOSIZING feature was removed in <https://trac.webkit.org/changeset/206119> (bug #162167). See bug 84186, comment 32 for more details.