Closed Bug 201811 Opened 21 years ago Closed 21 years ago

"font-size: smaller" does not work right

Categories

(Core :: CSS Parsing and Computation, defect)

Product:
Core
Component:
CSS Parsing and Computation
Platform:
x86
Linux
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: jwz, Assigned: dbaron)

References

Details

Attachments

(6 files, 1 obsolete file)

screen shot
7.62 KB, image/gif
Details
In order to compare apples to apples, the original testcase w/ 4.01 strict doctype
925 bytes, text/html
Details
1024x768 120DPI OS/2 SS of attachment 120348 (renders as expected)
6.58 KB, image/png
Details
SS of Mandrake Linux 9.0/KDE 120DPI 1024x768 - a different incorrect rendition
6.91 KB, image/png
Details
expanded testcase (trebuchet ms)
2.43 KB, text/html
Details
SS Mandrake 9.0/KDE 120 DPI 1024x768 of attachment 120358
16.38 KB, image/png
Details 
In the following document, the first "smaller" does not work
but the second one does:

    <style type="text/css">
      body { font-size: 10pt; }
    </style>

    <p>
    <div style="font-size: 10pt">
       MMMMMMMMMMMMMMMMMMMMM "10"</div>
    <div style="font-size: 9pt">
       MMMMMMMMMMMMMMMMMMMMM "9" (looks smaller)</div>
    <div style="font-size: 8pt">
       MMMMMMMMMMMMMMMMMMMMM "8" (looks smaller still)</div>

    <p>
    <div style="font-size: 10pt"> MMMMMMMMMMMMMMMMMMMMM "10"
      <div style="font-size: smaller">
        MMMMMMMMMMMMMMMMMMMMM "smaller" (looks same as "10", not "9")
        <div style="font-size: smaller">
          MMMMMMMMMMMMMMMMMMMMM "smaller" (looks same as "8"!)
        </div>
      </div>
    </div>

The first three lines are three different font sizes (10, 9, 8).
The next three lines are only two different sizes (10, 10, 8).
Should not both three-line groups look the same?

Adding
    <!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN"
              "http://www.w3.org/TR/html4/strict.dtd">
does not change the behavior.


--------------------

Mozilla 1.3
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.3) Gecko/20030314

Red Hat Linux release 8.0 (Psyche)
Linux 2.4.18-14 #1 Wed Sep 4 12:13:11 EDT 2002 i686 athlon i386 GNU/Linux


My Appearance/Fonts preferences say:

    Proportional Sans Serif 18
    Monospace default 16
    Allow documents to use other fonts
    Display resolution System setting

xdpyinfo says:

    dimensions:    1280x1024 pixels (433x347 millimeters)
    resolution:    75x75 dots per inch
    depth of root window:    24 planes

xset q says:

    Font Path:
      /home/jwz/.gnome2/share/cursor-fonts,
      unix/:7100,
      /home/jwz/.gnome2/share/fonts

RPMs:

    mozilla-1.3-0_rh8_xft
    mozilla-nss-1.3-0_rh8_xft
    mozilla-psm-1.3-0_rh8_xft
    mozilla-nspr-1.3-0_rh8_xft

    Xft-2.0-1

    XFree86-100dpi-fonts-4.2.0-72
    XFree86-75dpi-fonts-4.2.0-72
    XFree86-base-fonts-4.2.0-72
    XFree86-font-utils-4.2.0-72
    XFree86-truetype-fonts-4.2.0-72

    bitmap-fonts-0.2-2
    fontconfig-2.0-3
    ghostscript-fonts-5.50-7
    ttfonts-1.0-9
    urw-fonts-2.0-26
Attached image screen shot — 

    
    

    
  
So.. The font size tables in content/html/style/src/nsStyleUtil.cpp don't have
any jumps from 10px to 8px.  If anything, the problem from those would have been
that the sizes were 10px, 8px, 8px in that testcase.

Chris, Jaime is using an xft build... any idea whether this could be a font
selection/rounding problem in xft?

    
    

    
  
See also bug 158906
Blocks: 158906

    
    

    
  
I see this also on MacOS X (build 2003041108), so I don't think it's an xft problem.

Could there be some kind of rounding error in nsStyleUtil? (I don't quite know
enough about the code to know which methods to look at; is
FindNextSmallerFontSize making the decisions here?)

According to the DOM inspector, font-size: 10pt computes to 13.33px; and
font-size: smaller (within that div) computes to 13px.  Seems like "smaller"
should drop the size more than a third of a pixel?

    
    

    
  
FindNextSmallerFontSize is indeed making the decision here.  See
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsRuleNode.cpp#2001


    
    

    
  


  
Problem appears to go away if either not using a bitmapped font or not sizing
with points. Save this testcase to disk and adjust font-family selection and/or
vary your default font-family to see there seems to be a granularity issue with
bitmapped fonts that disappears when abandoning points for sizing. Besides that
effective DPI affects point sizes, the particular DPI chosen seems to impact
which of the three screenshot behaviors is exhibited.

    
    

    
  
It appears that nsStyleUtil::FindNextSmallerFontSize can make bad choices for 'font-size: smaller' depending on the DPI.  The root issue is that this method finds the next smallest font size by comparing values in _points_, while the HTML/CSS font size table is in _pixels_, and floors when converting from twips to points.  The loop in question is at 
http://lxr.mozilla.org/seamonkey/source/content/html/style/src/nsStyleUtil.cpp#430

for (index = indexMax; index > indexMin; index--) 
    if (fontSize > NSTwipsToFloorIntPoints (CalcFontPointSize (index, aBasePointSize, aScalingFactor, aPresContext , aFontSizeType))) 
     break; 

For me, CalcFontPointSize uses the last row of the  strict HTML/CSS table = { 9px,10px,13px,16px,18px...}  (I guess the row is based on your default font size?).  Now on my setup (1152x870, 96dpi), 10pt=13.33px, so the 'smaller' font size will be 13px, because it will be converted to size 9pt, and thus be smaller than the 10pt parent font size.  But, when the HTML/CSS size index is returned to SetFont() in nsRuleNode.cpp, it stores the size in twips since that's what nsStyleFont uses.  Then, whatever is choosing the actual font to draw evidently uses the closest match (13px->10pt), as opposed to the 'floor' point size (13px->9pt).  I don't know which is the right choice overall.

I ran through FindNextSmallerFontSize on a debug build a few times to check to see if there were any propagated rounding errors after all the twips/points/pixel conversions, but everything seems to work you would expect from the code.

Is there a good reason that FindNextSmallerFontSize needs to do its math in points?  To me, it seems that it would better to stay in twips, and pick smaller/larger font sizes by "rounding" the parent font size to the nearest HTML/CSS font size table entry, and then picking the next smallest entry.  But I don't know much about this, so I may be way off base.  (This would make the loop a little more complicated.)

    
    

    
  
(sorry about the **** formatting in the last comment)

On second thought, maybe it would make sense to use nsTwipsToCeilIntPoints in
the FindNextSmallerFontSize loop instead of the floor function?  

The floor function works fine for 'font-size: larger' since it converts the
HTML/CSS table entries to the smallest possible [sane] point size choice for
each pixel size, guaranteeing that 'font-size: larger' will in fact be larger. 
It seems we want the "opposite" behavior for 'font-size: smaller' (ie, convert
everything to the largest possible point size, and pick the one that's still
smaller).  

    
    

    
  
I think the best solution (which I described in another bug, somewhere), is that
'smaller' and 'larger' shouldn't snap to the values in the table, but should
instead extrapolate based on the ratio of the nearest values in the table
(although when smaller than the table, they might want to skip by 1px intervals).

    
    

    
  
dup of bug 72164?

    
    

    
  
Could be, yes... Hard to tell for sure till Jaime tries a patched build. ;)
Depends on: 72164

    
    

    
  

      
      
      
      

        Attached file
          
        new FindNextSmallerSize (obsolete)
        — 
      

    


  
So, here is a possible new FindNextSmallerFontSize that linearly interpolates
between HTML/CSS font size keywords.  If the parent font size is smaller
than 'x-small', then it just reduces the font size by 1px.

It doesn't extrapolate for font sizes larger than xx-large, since I'm
not sure what the right thing to do is. (80% decrease?)

It would be easy to make a new FindNextLargerFontSize from this.

Since FindNextSmallerFontSize would return font size in twips, some
headers and nsRuleNode.cpp would have to be slightly modified before making a
bona fide patch.

Does this look reasonable? 

(About the utility of this method: for me the end results are
barely distinguishable from just changing to NSTwipsToCeilIntPoints in
the original for loop.	Maybe my DPI setting is fortunate.)

    
    

    
  
Comment on attachment 120411 [details]
new FindNextSmallerSize


>      largerIndexFontSize = CalcFontPointSize(index+1, aBasePointSize, aScalingFactor, aPresContext, aFontSizeType);
>      smallerIndexFontSize = CalcFontPointSize(index-1, aBasePointSize, aScalingFactor, aPresContext, aFontSizeType);

Hmm.. what about the size at |index|?  Is there a reason we're not using that?

    
    

    
  
not sure what you mean....  indexFontSize gets set to the size at |index| 
coming out of the loop just above, and is used in the ratio calculations below.



    
    

    
  
Comment on attachment 120411 [details]
new FindNextSmallerSize

Ah, nevermind. I was misreading this...

        Attachment #120411 -
        Flags: superreview?(dbaron)

        Attachment #120411 -
        Flags: review?(bzbarsky)

    
    

    
  
uh, before submitting for review it should have some kind of behavior for font 
sizes larger than xx-large.  right now it will just drop to xx-large.  I was 
hoping for some input on this one, but I suppose decreasing to 80% of current 
size would be reasonable?

I should also submit a real patch, with a similar FindNextLargerSize... 

    
    

    
  
Comment on attachment 120411 [details]
new FindNextSmallerSize

Yeah, a real patch would be nice... ;)

Currently, the code that handles this return value is at
http://lxr.mozilla.org/seamonkey/source/content/base/src/nsRuleNode.cpp#2014 --
it just divides the font size by 1.5, looks like...  It looks like the check
for that and the lower end of the table can both be removed there if you handle
that in FindNextLargerFontSize.

I assume you know how to set the review flags once you post a patch you're
happy with. ;)

        Attachment #120411 -
        Flags: superreview?(dbaron)

        Attachment #120411 -
        Flags: review?(bzbarsky)

    
    

    
  
posted a patch in bug 72164, since that's what the patch really addresses...

    
  

        Attachment #120411 -
        Attachment is obsolete: true

    
    

    
  
The fix for bug 72164 should fix this one too (it does on OS X).  Maybe jwz or
somebody else wants to try a Xft build to make sure it's for them?

    
    

    
  
Mozilla/5.0 (X11; U; Linux i586; en-US; rv:1.4.1) Gecko/20031009 (Fedora 0.95
1.4.1-7 rpm; about: buildconfig portions: --enable-default-toolkit=gtk2
--disable-freetype2 --enable-xft)

At 1024x768 96 DPI, I don't see this now. In attachment 120348 [details] testcase, the
three bottom rows are discrete sizes, while the top two rows, 9pt and 10pt, are
identical, and match row 4, which looks like a granularity shortcoming of the
font server having nothing to do with our application of the rule font-size:
smaller. Is this not fixed by bug 72164?

    
    

    
  
Yeah, this is fixed.
Status: NEW → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED

          You need to log in
          before you can comment on or make changes to this bug.
        






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: