The 'bolder' and 'lighter' keywords don't always work. They are supposed to guarentee that we pick a lighter or bolder font-weight, but we don't always. See: http://www.hixie.ch/tests/adhoc/css/fonts/weight/002.xml for an example. Tested using Netscape Commercial Trunk build 2001080110 on Windows 2000. See also bug 77882.

Note that Opera pass that test (or rather, an HTML equivalent) perfectly.

Moving to mozilla1.1. Engineers are overloaded!

Bulk moving from Moz1.1 to future-P1. I will pull from this list when scheduling work post Mozilla1.0.

cc'ing myself

Assigning pierre's remaining Style System-related bugs to myself.

Reconfirmed using FizzillaCFM/2002070913. Setting All/All.

*** Bug 223550 has been marked as a duplicate of this bug. ***

I still see this bug in a FF3 nightly on linux and a xulrunner build.

I still see this bug with the very latest FF3 tree on Linux [Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv:1.9pre) Gecko/2008050719 Minefield/3.0pre].

This proved to be pretty simple to fix, with some caveats. The attached patch passes Hixie's test linked from here, + its own reftests, and doesn't break any reftests. Caveat #1: Thebes does not implement nsIFontMetrics::GetFontHandle, which is the obvious way to tell whether a change to the .weight field of an nsFont has actually resulted in the selection of a different font. The code in this patch is instead relying on the assumption that changing font weight is likely to change the font's max-advance metric. In local testing this works great for proportional fonts, but not for monospace fonts (naturally enough) -- this is still an improvement on the present state, but I would be grateful for better ideas. Caveat #2: The reftests included in this patch are weaker than Hixie's original. I see no way to make them stronger without relying on a particular font being installed on the testing system, but they did miss a bug in my first attempt at a fix, that Hixie's test caught. Caveat #3: contra the previous code, I make no use whatsoever of weight values that are not multiples of 100. This is consistent with my reading of CSS2.1 but not consistent with the discussion in bug 77882. I put more detailed notes there - see especially my (bug 77882 comment 7) and Hixie's (bug 77882 comment 2). I'm inclined to say that this caveat, unlike possibly the other two, should not block this bug; rather, if non-multiple-of-100 weights are sometimes appropriate, we can leave bug 77882 open after this lands and fix it there.

I think caveat #1 is a problem, and you need to expose something from Thebes rather than relying on the max-advance trick.

That said, the other thing I'm not sure about is really a spec question. Consider the following <div style="font-weight: bold"><div style="font-weight: lighter">京B</div></div> and suppose that we further have: div { font-family: Bookman, MS Song; } Now, Bookman has a weight between 400 and 700, but MS Song (let's presume) does not. Should the 京 be weight 400 or 700? (Note that I'm not sure whether Bookman Demi is 500 or 600, but I'm guessing that it's 600, which, when absent, should be rounded to 700; if it's actually 500, then replace "bold" above with "normal" and "lighter" above with "bolder".) In other words, is the lighter / bolder supposed to be applied element-by-element or character-by-character?

I would further note that I was under the impression that this bug was already fixed on at least some platforms, in the font selection code.

Your character-by-character/element-by-element example gets at a deeper problem - every value coming from a Thebes nsIFontMetrics is only true for the first font in the underlying font group. This being so, I suppose I can just expose mFontGroup->GetFontAt(0) as nsThebesFontMetrics' version of GetFontHandle(). It wouldn't be any more wrong than what we're already doing. I don't see how this could be fixed in font selection - by the time we get there the request for lighter/bolder has become indistinguishable from a request for some numeric font weight...

In the past what we passed down to the font selection code was a number like 402 which could be decomposed into 400 + bolder + bolder, or 699, which means 700 + lighter.

I messed around with a GetFontHandle() approach but it doesn't work, for no apparent reason.

This is me thinking out loud. Several, but not all, of the system-specific font selection routines have code to decompose numbers like 402 into 400+bolder+bolder and then do something with that, but the somethings are not consistent with each other and most of them are not consistent with CSS2.1 either, as far as I can tell. One notable quirk is that some of them, if they can't find a different weight in the specified direction from the base weight, try the opposite direction, which as far as I can tell is just plain wrong. Also, the font-selection routines that do this decomposition all attempt to do synthetic boldface under some conditions, especially when the font family has no boldface at all. It seems silly to implement weight selection at the system-specific level, if only because it means the behavior gets coded N times and they aren't necessarily consistent with each other. Making Thebes' GetFontHandle() return meaningful values is more of a memory management headache than I want to get into. The low level code (at least on some platforms) seems to deal in names and treat system font handles as transient objects. Possible way forward: add a nsI(Thebes)FontMetrics interface that returns a nine-element array of numbers, exposing the low-level code's choice of which CSS numeric weights are visually distinct in that font family -- the numbers in the array are just cookies, but visually distinct weights have distinct numbers. StyleUtils uses that to pick a numeric weight when lighter/bolder are used, and we scrap the base/offset weight encoding. Whether or not synthetic bold is used remains up to the low-level code.

(In reply to comment #17) > are not consistent with CSS2.1 either, as far as I can tell. One notable quirk > is that some of them, if they can't find a different weight in the specified > direction from the base weight, try the opposite direction, which as far as I > can tell is just plain wrong. Also, the font-selection routines that do this It seems perfectly reasonable if the base weight isn't available either.

(In reply to comment #18) > [Trying the opposite direction] seems perfectly reasonable if > the base weight isn't available either. Yah, but I'm pretty sure it's not limited to that. For clarity, the thing that seems "just plain wrong" to me is the possibility that "bolder" might under some circumstances produce a lighter weight than unstyled, or "lighter" a bolder weight.

Comment on attachment 323226 [details] [diff] [review] possible patch for this and #77882 retracting review request.

It's probably worth asking Stuart and Vlad what they think of the two possible approachs (gfx code, character-by-character; style code, element-by-element).

> It's probably worth asking Stuart and Vlad what they think of the two possible approaches Ok, I've added them to the cc: list. Stuart, Vlad - could you please look at this bug, especially comments 12, 14, 17, and 21, and let me and dbaron know what you think?

discussion on IRC converged on doing it character-by-character in gfx but changing the way style communicates the lighter/bolder chain so that gfx can get it right. also of note is that most font backends do implement lighter/bolder (with bugs) but pango (Linux) doesn't, and that gfx needs to communicate its eventual choice back up, for getComputedStyle()'s sake [it is sufficient to communicate the choice for the first font in the element].

And see also bug 3512 comment 43.

Discussing on IRC with stuart, changes needed in gfx to correctly handle bolder/lighter compounding correctly: 1. Handle a "chain" of bolder/lighter changes instead of a single base weight plus delta value. Example: 32-bit integer, lower 4 bits = base weight ([1,9]), upper 28 bit = series of 2 bit values 10=lighter, 11=bolder 2. Resolved weight is calculated using array of available weights for a given font family. a. resolve initial base weight to available weights using CSS rules (bold weights map to bolder faces, lighter weights map to lighter faces). b. resolve each bolder/lighter chain using available weights, set flag when bolder is set but resolved weight < 600, clear this when lighter is set c. if flag is set and the resolved weight is < 600, computed weight is set to 600, otherwise to the resolved weight [for synthetic bolding] Examples: FontA (400, 700) 300 + bolder ==> 700 800 + bolder ==> 700 700 + bolder ==> 700 700 + bolder + lighter ==> 400 700 + bolder + bolder + lighter ==> 400 FontB (400, 700, 900) 300 + lighter ==> 400 300 + lighter + bolder ==> 700 700 + bolder ==> 900 700 + bolder + lighter ==> 700 700 + bolder + bolder + lighter ==> 700 FontC (400) 700 ==> 600 (synthetic bolding) 700 + bolder ==> 600 (synthetic bolding) 700 + bolder + lighter ==> 400 100 + lighter + bolder ==> 600 (synthetic bolding) 3. Rendering occurs based on <resolved weight, flag>, since the specifics of synthetic bolding is platform-specific, in some cases we need to do it manually.

Tweak 2b. set flag when bolder is set but no bolder weight is available and resolved weight < 600

So, it actually occurred to me today while writing http://lists.w3.org/Archives/Public/www-style/2008Jun/0151.html that maybe the spec *meant* for this to be a count rather than a sequence, and we don't need to do the chain thing. See that message and http://lists.w3.org/Archives/Public/www-style/2008Jun/0147.html .

Taking this off wanted1.9.1+ list as we are still waiting for a decision from the CSS WG.

So...? They've decided yet?

(In reply to comment #30) > So...? They've decided yet? Not yet. The place for asking this question is the www-style mailing list, not here. The proposed solution was discussed at the last meeting in June, you can read the discussion in the mailing list archives.

I no longer work for Mozilla, I am deassigning myself from bugs I have no intention of working on as a volunteer.

The CSS working group did accept the new behavior. I have a patch for it in my patch queue, since writing the patch was the easiest way to verify that a test was correct.

And the patch fixes these pairs of tests in the CSS 2.1 test suite (which we should probably add as reftests): http://test.csswg.org/suites/css2.1/20101001/html4/font-weight-bolder-001.htm http://test.csswg.org/suites/css2.1/20101001/html4/font-weight-bolder-001-ref.htm http://test.csswg.org/suites/css2.1/20101001/html4/font-weight-lighter-001.htm http://test.csswg.org/suites/css2.1/20101001/html4/font-weight-lighter-001-ref.htm http://test.csswg.org/suites/css2.1/20101001/xhtml1/font-weight-bolder-001.xht http://test.csswg.org/suites/css2.1/20101001/xhtml1/font-weight-bolder-001-ref.xht http://test.csswg.org/suites/css2.1/20101001/xhtml1/font-weight-lighter-001.xht http://test.csswg.org/suites/css2.1/20101001/xhtml1/font-weight-lighter-001-ref.xht The patch (no tests yet) is here: http://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/8d54e6e9abd3/bolder-lighter-style There might be some additional code in gfx/ that could be removed.

Here's the code; still need to import the reftests, though. Given the CSS 2.1 status (and the fact that this simplification might have to be undone from the spec if we don't get this in soon), I think we should try and get this in to Firefox 4.

Comment on attachment 488444 [details] [diff] [review] patch Requesting approval2.0, for the following reasons: This is implementing a change in the latest draft of CSS 2.1 that significantly simplifies the spec; it makes it significantly easier to implement 'bolder' and 'lighter' and also means that implementing them requires no platform-specific code, which reduces the burden of porting. It also provides a more consistent behavior for common uses of 'bolder', like the HTML <b> element, in the presence of font families that have a large number of barely-distinguishable weights, which may start to become more common given downloadable fonts. (The new text is at http://www.w3.org/Style/css2-updates/css2/fonts.html#propdef-font-weight ; it makes 'bolder' and 'lighter' compute a new value based only on the inherited value.) I'd like to get this change in ASAP for two reasons: (1) I'm concerned that if we don't get the change in soon, CSS 2.1 will be forced to revert the change for exiting CR, and the Web might end up permanently stuck with the more complicated rules. (2) It's fairly low-risk today, but as more people start using high-quality downloadable fonts, it potentially becomes more risky, since it would be more likely to change behavior on Web pages.

This is the patch for the test I didn't get around to adjusting earlier.

I also made some mistakes in the adjusting of reftest references in the first patch, which I believe I fixed in: http://hg.mozilla.org/users/dbaron_mozilla.com/patches/rev/13e8f9ef7b83

Even with the correct adjustments, weightmapping-458 still fails on Windows due to what looks to be a line-height difference. I think the problem is that the current structure of the tests is inconsistent between test and reference about whether the final font is set on an inline or a block; making span a block fixes that and means the outer font settings don't influence the line layout.

Comment on attachment 489205 [details] [diff] [review] fix for synthetic-variations reftest Looks right. With the elimination of the delta weights (e.g. 698, 402), this test is less interesting.

http://hg.mozilla.org/mozilla-central/rev/864ad777dba7 http://hg.mozilla.org/mozilla-central/rev/cf61d18f6210

I noticed this based on code coverage data from our test suite. This code should have been removed in the earlier patches on this bug.