Bug 12530 - CSS3: Support the font-stretch property
Summary: CSS3: Support the font-stretch property
Status: RESOLVED CONFIGURATION CHANGED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: 420+
Hardware: Mac OS X 10.4
: P2 Normal
Assignee: Nicholas Shanks
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2007-02-01 07:43 PST by Nicholas Shanks
Modified: 2022-09-04 13:49 PDT (History)
28 users (show)

See Also:


Attachments
patch v 1.0 (209.25 KB, patch)
2007-02-01 17:51 PST, Nicholas Shanks
mrowe: review-
Details | Formatted Diff | Diff
patch v 1.1 (226.31 KB, patch)
2007-02-01 21:04 PST, Nicholas Shanks
no flags Details | Formatted Diff | Diff
patch v 1.2 (226.32 KB, patch)
2007-02-01 21:42 PST, Nicholas Shanks
hyatt: review-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Nicholas Shanks 2007-02-01 07:43:46 PST
Soon to be attached is my patch to add font-stretch support to WebKit, originally part of the patch for bug #6484 to add font-weight support, but broken out per request of dave hyatt.
Comment 1 Nicholas Shanks 2007-02-01 17:51:23 PST
Created attachment 12865 [details]
patch v 1.0

This patch has two known bugs:
1) The font-stretch property doesn't appear in the Web Inspector palette. I don't know where to fix this.
2) A font which has only Regular and Condensed faces (such as those used in the layout test) will fail when it encounters code such as:

<p style="font-stretch: wider"> foo <span style="font-stretch: narrower"> bar </span> baz </p>

The "bar" in the above is supposed to be Condensed, but at present is not. This bug is encountered when the FontDescription asks for a stretch that cannot be met and the 'narrower' value is calculated from this unmet stretch, and not the actually used stretch. I am unable to come up with a means of telling the FontDescription the bad news. I tried adding member vars to FontPlatformData but couldn't find a solution.
(hixie and dbaron confirmed that my expected behaviour is what should ocour, and the present be)


The code that selects "a closer weight regardless of traits" is needed for situations where say you want a Extended Bold Italic but only Regular, Bold and Italic are available. In this case, The Bold will be chosen, oblique synthesised and stretch ignored. It could have been dealt with in the Traits hierarchy below with some more complicated code, but since I intend to remove the NSBoldFontTrait from that with my font-weight patch anyway, saw no need to add code here and take it away again tomorrow. The more complicated code (that was actually a bit buggy with Italic 600 weight) can be seen in my first patch on bug #6484.

Also, requests for condensed and extended traits are ignored when finding a first match. Without this, requests with those traits could fail to match even the fall-back fonts, and thus lock up the browser.

There is currently a regression somewhere in the tree that causes Lightest weights to be selected instead of Regular. The code touched by this patch is never reached and so this problem is not fixed.
Until I fix that regression (with the aforementioned font-weight patch) you'll need to disable the fonts American Typewriter Light and Helvetica Neue Light/UltraLight to get the test to pass. I could have provided a different expected result but then people who don't have these fonts won't pass instead, and I would have to update the results along with the font-weight patch.
Comment 2 Darin Adler 2007-02-01 18:10:49 PST
(In reply to comment #1)
> 1) The font-stretch property doesn't appear in the Web Inspector palette. I
> don't know where to fix this.

CSSComputedStyleDeclaration.cpp
Comment 3 Mark Rowe (bdash) 2007-02-01 19:56:18 PST
Comment on attachment 12865 [details]
patch v 1.0

I'm going to r- this for a few reasons, most of which I've mentioned on IRC:
* We can't land a failing layout test.
* Your patch has a few coding style issues (tabs in a few lines, and two one-line-ifs which should probably be merged and indented on to two lines)
* It'd be good if you could address the "Web Inspector" issue you mention when this is landed so that it doesn't get overlooked.

I'm far from being an expert on our CSS code, but the rest of the change looks reasonable to me.
Comment 4 Nicholas Shanks 2007-02-01 21:04:05 PST
Created attachment 12866 [details]
patch v 1.1

Addressed bdash's points, added web inspector fixes to CSSComputedStyleDeclaration and inspector.js
Tests now pass without having to disable fonts, or fail if you don't enable them. Can't have your cake and eat it ;-)
Comment 5 Nicholas Shanks 2007-02-01 21:42:11 PST
Created attachment 12867 [details]
patch v 1.2

killed one tab monster, and two ? ran away from their parents
Comment 6 Mark Rowe (bdash) 2007-02-01 22:28:52 PST
Comment on attachment 12867 [details]
patch v 1.2

I applied this patch to land and it introduces two failures: fast/css/computed-style.html and fast/css/font-stretch.html.  Marking as r- so that they can be addressed.
Comment 7 Mark Rowe (bdash) 2007-02-01 23:52:18 PST
Comment on attachment 12867 [details]
patch v 1.2

This patch wasn't to blame.  Landed in r19350.
Comment 8 Mark Rowe (bdash) 2007-02-05 17:21:22 PST
Dave has asked me to roll this patch out for the moment.  I'll be doing that shortly.
Comment 9 Dave Hyatt 2007-02-05 17:28:28 PST
Comment on attachment 12867 [details]
patch v 1.2

I am actually minusing this review for a number of reasons (and this patch is being rolled out of the tree).

font-stretch was  actually dropped from CSS2.1, and so it exists now only in a CSS3 draft.  This property should be prefaced with the -webkit- prefix, since it is really CSS3 now and not CSS2.1.

font-stretch needs to work all the time, just as bold/italic do.  That means it isn't enough just to support different font variants when they do exist.  You also need to support font synthesis.

Normally I would have left this patch in and just filed bugs suggesting that this work be iterated on, but given that we've locked down for stabilization, I don't want to ship a half-implemented version of this property.
Comment 10 Mark Rowe (bdash) 2007-02-05 18:36:41 PST
Rolled out in r19421.
Comment 11 Nicholas Shanks 2007-02-06 08:43:10 PST
I disagree with Dave on the -webkit- prefix, this property is well defined in CSS2 [a Candidate Recommendation] and there are no intentions to change it currently for CSS3. The only reason it is not yet present in the CSS 2.1 draft is because no-one has currently implemented it. I would consider this property to be as stable as the CSS3-selectors module which is already implemented. CSS 2.1 tries to define the subset of CSS2 that is reliably implemented. It does not attempt to supersede CSS2 (and thus I consider it to be poorly named, CSS2 Tiny may have been better!)

I agree that synthesis would be great, but disagree that it would be necessary for this feature to be considered supported. There is no such requirement in the specifications, and given the way font-weight algorithms are suggested (matching to nearest available face) I would say the current patch is exactly what the specs define, and is not "half-implemented".
I have font-stretch synthesis on my list of things to do for after Safari 3 branches.

You further say that it would have to work 'all the time, just as bold/italic do' but I should point out that synthetic bold only works at low font sizes. It is next-to useless at sizes above 36px (whether zoomed in, or via CSS), it cannot emulate lighter weights nor heavier-than-bold weights. Yet all these misgivings are not considered enough of a problem to discontinue support for the entire font-weight property.

It would be of significant detriment to the web if some later versions of Safari 3 had support for this, but earlier ones did not support it at all, because many web developers and CSS compliance charts only go by major version numbers, and many web users won't upgrade with minor updates.

Lastly, I thought that we had until Wednesday the 7th before new feature lock-out.
Comment 12 Nicholas Shanks 2007-02-08 05:22:07 PST
iCab 3.0.3 supports font-stretch by simulation, and without vendor prefix.
Comment 13 Eric Seidel (no email) 2007-10-01 09:27:26 PDT
Looks like this bug has been (sadly) forgotten.  And also might be incorrectly named (if font-stretch is really a CSS2 property).
Comment 14 Nicholas Shanks 2008-04-04 14:27:23 PDT
Adding mitz since he fixed up bug 6484 this week.
Comment 15 Simon Fraser (smfr) 2011-06-23 14:57:33 PDT
http://www.w3.org/TR/css3-fonts/#font-stretch-prop
Comment 16 Nicholas Shanks 2011-10-03 13:26:04 PDT
For the record, font-stretch finally got implemented for Firefox 9:
http://hacks.mozilla.org/2011/09/introducing-aurora-9/
https://bugzilla.mozilla.org/show_bug.cgi?id=3512
Comment 17 Chris Dumez 2012-10-02 02:30:51 PDT
Any update on this?

font-stretch is still present in the latest spec (http://www.w3.org/TR/css3-fonts/#font-stretch-prop) and it is still unimplemented in WebKit.
Comment 18 Radar WebKit Bug Importer 2015-05-04 09:14:11 PDT
<rdar://problem/20800191>
Comment 19 Ahmad Saleem 2022-09-03 14:46:47 PDT
If I am not wrong, this is now supported and might have landed through other patches:

https://github.com/WebKit/WebKit/commit/54a57c16f65c2229816454d3e1390db5f15c0ef3

https://github.com/WebKit/WebKit/commit/89624119faa70550f3f0eb1c9d796c33a0259380

https://github.com/WebKit/WebKit/commit/824773817acf5babfae406e2730a114e0c3a6ae8

etc.

I think this can be marked as "RESOLVED CONFIGURATION CHANGED" or Duplicate of main bug..

I will leave it for someone else to decide and mark this accordingly. Thanks!
Comment 20 Alexey Proskuryakov 2022-09-04 13:49:42 PDT
Basic functionality tried via https://developer.mozilla.org/en-US/docs/Web/CSS/font-stretch does work.