93843 - <font> suppresses the font-size property

Status: RESOLVED DUPLICATE of bug 72164

(Core :: CSS Parsing and Computation, defect)

Description

[rbs]

BUILD ID: Tip of the trunk. Not sure if this is a regression as I haven't
tested earlier builds.

STEPS TO REPRODUCE:
1) To clearly see the problem, set a sufficiently large default font-size in 
your prefs.js (or default_prefs.js if using viewer):

user_pref("font.size.variable.x-western", 100);

2) Create a sample document containing:
<p>
<span style="font-size:16px">
  <font size="+1">
    text
  </font>
</span>
</p>

EXPECTED RESULTS:
'text' should render in a size slightly above 16px

ACTUAL RESULTS:
'text' is rendered in 100px (the value set in the prefs)

ADDITIONAL INFO:
1) the bug appear with any non-zero value other than '+1' (it can be a positive 
or negative value).

2) 'text' is rendered in 16px with the change:
- <font size="+1">
+ <font size="+0">

Comment 1

[rbs]

More weirdness

<span style="font-size:50px">
  50px
  <span style="font-size:smaller">
     to smaller
     <span style="font-size:larger">
        to larger  <!-- bug here: nothing happens, the size doesn't change -->
     </span>
  </span>
</span>

Adding keyword:qawanted because it is getting suspicious. Other combinations may 
be failing.

Comment 2

[Hixie (not reading bugmail)]

That last bug is bug 72164, font-size:smaller doesn't extrapolate values. (Well,
it's the reverse case, but it's the same bug.)

The original bug is probably the same code, don't we map +1 and similar values
to the same code which handles 'larger' and 'smaller'? Either way, I guess the
bug is "simply" that our relative sizing logic is bad, an extension of bug 
72164. I doubt it is anything specific to the <font> element.

Removing qawanted on the assumption this answers the questions. Feel free to 
readd it if you want specific test cases.

Comment 3

[rbs]

Dup... Indeed, they boil down to the same code path.


*** This bug has been marked as a duplicate of 72164 ***

Comment 4

[rbs]

I have got more insight into what is happening with my first description.
The problem is originating from MapAttributesIntoRule() inside 
mozilla/content/html/content/src/nsHTMLFontElement.cpp

Keeping in mind that
eHTMLUnit_Integer is associated with:
      ...  -2,  -1,  +1,  +2, ...
eHTMLUnit_Enumerated is associated with:
      1, 2, 3,

Lines 255 and 256 below show that, for example, 
  <font size="+1">
     <font size="+1">

always resolve to:
  <font size="4">
     <font size="4">

instead of:
  <font size="4">
     <font size="5">

===================

218 static void
219 MapAttributesIntoRule(const nsIHTMLMappedAttributes* aAttributes,
220                       nsRuleData* aData)
221 {

[...]

240     // pointSize: int, enum
241     if (font.mSize.GetUnit() == eCSSUnit_Null) {
242       aAttributes->GetAttribute(nsHTMLAtoms::pointSize, value);
243       if (value.GetUnit() == eHTMLUnit_Integer ||
244           value.GetUnit() == eHTMLUnit_Enumerated) {
245         PRInt32 val = value.GetIntValue();
246         font.mSize.SetFloatValue((float)val, eCSSUnit_Point);
247       }
248       else {
249         // size: int, enum , 
250         aAttributes->GetAttribute(nsHTMLAtoms::size, value);
251         if ((value.GetUnit() == eHTMLUnit_Integer) ||
252             (value.GetUnit() == eHTMLUnit_Enumerated)) { 
253           PRInt32 size = value.GetIntValue();
254           if (size) {
255>>>>>>       if (value.GetUnit() == eHTMLUnit_Integer) // int (+/-)
256>>>>>>         size = 3 + size;  // XXX should be BASEFONT, not three
257               
258             size = ((0 < size) ? ((size < 8) ? size : 7) : 1); 
259             font.mSize.SetIntValue(size, eCSSUnit_Enumerated);
260           }
261         }
262       }
263     }
264 

[...]

294   }

=========================

Since the parent font can be anything at this stage, it looks like the fix for
this is to preserve things, and use SetIntValue(size, eCSSUnit_Integer) or 
SetIntValue(size, eCSSUnit_Enumerated) as appropriate. Then, let the cascading 
code in nsRuleNode.cpp handle this special case when the parent font is known. 
Note that this will be an internal abuse of the font-size since '+/- int' is not 
a valid CSS font-size that can be set externally by users. However, it will 
remain internal and I don't see another simpler way to fix the problem.

Comment 5

[rbs]

What a messy element... Just in case, I made a trip to the HTML4 spec to 
double-check, and here is what I read over there:

"The BASEFONT element sets the base font size (using the size attribute). Font 
size changes achieved with FONT are ***relative to the base font size*** set by 
BASEFONT. If BASEFONT is not used, the default base font size is 3." (emphasis 
is mine.)

In other words, the current behavior is correct as per the spec. The parent font 
doesn't count. (It just smells like a contortion put in the spec to accommodate 
bugs that were present in existing implementations.)