131454 - incorrect inheritance of font-size

Status: VERIFIED FIXED

(Core :: CSS Parsing and Computation, defect)

Description

[Matt Patterson]

From Bugzilla Helper:
User-Agent: Mozilla/5.0 (Macintosh; U; PPC Mac OS X; en-US; rv:0.9.9) Gecko/20020310
BuildID:    2002031005

Somewhere between 0.9.8 and 0.9.9 font-size seems to have stopped being
inherited properly.In the testcase I specify a font-size: 12px on the body
element and a font-size: 1em on the p element. The text within the p appears
with an apparent font-size of 16px -- my default. This is different behaviour
from 0.9.8 and earlier (all the way back to 0.8 at least)

Reproducible: Always
Steps to Reproduce:
1.load the testcase, or other page with font-size cascaded using em.

Comment 1

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Eek.  This looks like it's somehow related to the code that deals with
font-family stuff.  I could've broken it with my zoom changes, or something else
could have.  rbs, any ideas?

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: testcase that shows the source of the problem

The problem is related to having a fully specified font struct on the style
context.

Comment 3

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch fixing bug

The cause of the problem was my optimization for lazy calls to the parent
contexts' GetStyleData.  CheckSpecifiedProperties doesn't need, for any other
purposes (at least I hope not), to compute whether 'em' or similar units cause
inheritance.  However, my optimization depended on it doing that for
'font-size', where 'em' means inheritance from a parent context rather than
from another or the same struct associated with the same context.  I *think*
this is all that's necessary to fix these issues, and it certainly fixes this
bug.

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The patch that I mentioned that caused the problem was the patch in bug 113098.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Comment on attachment 74550 [details] [diff] [review]
patch fixing bug

make sense... r=bzbarsky

Comment 6

[David Hyatt]

sr=hyatt

Comment 7

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I should test a rule with something like:

 font: menu;
 font-size: 2em;

Comment 8

[Asa Dotzler [:asa]]

Comment on attachment 74550 [details] [diff] [review]
patch fixing bug

a=asa (on behalf of drivers) for checkin to the 1.0 trunk

Comment 9

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Comment on attachment 74550 [details] [diff] [review]
patch fixing bug

>@@ -761,7 +762,7 @@
>         (family == NS_STYLE_FONT_LIST) ||
>         (family == NS_STYLE_FONT_FIELD) ||
>         (family == NS_STYLE_FONT_THEME))
>-      return nsRuleNode::eRuleFullMixed;
>+      return nsRuleNode::eRuleFullReset;
>   }
>   return nsRuleNode::eRuleUnknown;
> }

I looked into the implementation of the 'font' shorthand and realized
this change is wrong, so I'm replacing it with:

@@ -761,6 +795,8 @@
	 (family == NS_STYLE_FONT_LIST) ||
	 (family == NS_STYLE_FONT_FIELD) ||
	 (family == NS_STYLE_FONT_THEME))
+      // Mixed rather than Reset in case another sub-property has
+      // an explicit 'inherit'.   XXXperf Could check them.
       return nsRuleNode::eRuleFullMixed;
   }
   return nsRuleNode::eRuleUnknown;

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Fix checked in 2002-03-18 17:32 PST.

Comment 11

[Hixie (not reading bugmail)]

verified using testcase

Comment 12

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

This fix wasn't quite enough.  I forgot percents.  See bug 132381.