Closed Bug 84398 Opened 23 years ago Closed 23 years ago

Back-end for the smarter support of the 5 basic CSS generic fonts

Categories

(Core :: CSS Parsing and Computation, defect)

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

Tracking

()

Status:
RESOLVED FIXED
Milestone:
mozilla0.9.5

People

(Reporter: rbs, Assigned: rbs)

References

Details

(Keywords: css2, fonts, Whiteboard: [Hixie-PF])

Attachments

(16 files)

ongoing patch in content
18.12 KB, text/plain
Details
ongoing patch in layout
18.88 KB, text/plain
Details
ongoing patch in gfxwin
10.95 KB, text/plain
Details
patch in layout v2
18.83 KB, text/plain
Details
patch in content v2
34.25 KB, text/plain
Details
patch in gfxwin v2
15.47 KB, text/plain
Details
Full source of nsRuleNode.cpp for easy reading of the changes as a whole
164.01 KB, text/plain
Details
patch in content v3 to fix a flaw in EnsureGenericFont (hop correctly at each step and don't use a pointer for the parentFont to avoid overwriting ealier data)
34.63 KB, text/plain
Details
changes in content v4 (full local picture for readability)
20.48 KB, text/plain
Details
final patch in content - v5
34.00 KB, text/plain
Details
final patch in layout - v3
20.24 KB, text/plain
Details
final patch in content v5' (I picked the wrong diff.txt earlier)
35.98 KB, text/plain
Details
ongoing spec compliant patch in content - implement font-size-adjust
12.29 KB, text/plain
Details
screenshot: example when font-size-adjust is disabled
10.70 KB, image/gif
Details
screenshot: example when font-size-adjust is enabled
17.11 KB, image/gif
Details
testcase used for the screenshots
1.26 KB, text/html
Details 
The current support of the default fonts mostly mimics what is done in
the old Nav. A new system is needed to support:
- multiple lists of default fonts for the 5 basic CSS generic fonts
- different attributes (e.g., different default font-size
and prefs for minimum font-size) for each of the generic fonts.

I will attach an ongoing patch that is ultimately aimed at this purpose. The 
patch brings about some improvements the style system by unifying the handling 
of the default variable font and the default fixed font. This unified approach 
provides a foundation for a cleaner support of the other generic fonts in a 
more convenient pref-controlled manner with no extra performance side-effects.

    
    

    
  


  

    
    

    
  


  

    
  
Blocks: 74186

    
    

    
  
Hope you folks can warn me before I get astray here...

So this bug came out of necessity, following my attempt at adding complete 
support for the 5 default generic fonts, and with individual, pref-controlled 
attributes like a preferred font-size for each generic font family, and cut-off 
values for different minimum font-sizes specific to each font.

Background:
In the current code, nsStyleFont has two nsFonts to mimic the old Nav behavior: 
one for the default variable font, and the other for the default fixed font. All 
the CSS computations are applied to these two fonts in parallel, and once the 
Style System determines that it is the fixed font that has be used, it 
overwrites the default variable font with the default fixed font:
[...]
-
-  if (font->mFlags & NS_STYLE_FONT_USE_FIXED)
-    font->mFont = font->mFixedFont;
[...]

So if the same logic was to be followed for the other 5 basic CSS generic fonts, 
then nsStyleFont would need 5 additional nsFonts; All of these will be updated 
in parallel, and once it is determined which font is really in effect, then the 
primary nsFont will be overwritten with that one. Need I take more space in this 
little bugzilla window by saying that this would hard to swallow?!

Based on this observation, what I am doing in the patch is to determine the font 
to be used at _initialization_, and cascade on that font. The idea is this: no 
matter what is happening with the CSS cascade (e.g., inheritance), you need a 
concrete fully resolved font to start with. And if this font is a generic font, 
the attributes can be initialized based on the pref values that are cached in 
the presentation context. Hence the ensuing CSS cascade can be based directly on 
that font and should compute the same values as the old code. So in principle, 
it should be faster and save one nsFont per unique nsSyleFont, and allows 
supporting other generic fonts at no extra cost. I have attached preliminary 
patches as a proof of concept (WIP, tested on Win32 -- need dummy prefs...).

    
    

    
  
What I described earlier is too good to be true. The problem is: if the resolved 
font data are not cached and a descendant child context later in the tree wants 
to inherit from it, where is the ancestor data going to come from? Indeed, a 
child can inherit from one generic font data, while another child inherits from 
another different generic font data.

Example:
<body> 
  <!-- here, the default font is the variable font from the presentation -->

  ...

  <p style="font-family:sans-serif; font-size:50%">
     <!-- here, the current font should be at 50% of the default size of 
          the _sans-serif_ font from the presentation context, bypassing
          the serif font from the parent context -->
     ...

     <span style="monospace; font-weight:bold; font-size:50%">
     <!-- here, the current font should be at 25% of the default size of 
          the _monospace_ font from the presentation context, bypassing
          the sans-serif or serif fonts from the anscestor contexts -->
       ...
     </span>

     <span style="cursive"> ... and so on ... </span>
  </p>
</body>

In other words, all the currently non-resolved non-explicit properties have to
cascade all the way down either from the presentation context or from an 
ancestor context for which the cascade was already done.

So, how to fix this bug?

1) Either declare placeholders for all the expected generic fonts and apply the 
cascade on these font data. This is what the existing code is doing, but this 
doesn't scale well as I noted above.

2) Another approach could be to lazily apply the cascade with a backtracking 
mechanism as follows: when a generic font is encountered, backtrack until an 
ancestor with the same generic font name (possibly up to the root style context 
where default values will come from the presentation context); And re-apply the 
cascading rules from there without caching the intermediate values in the 
context tree or the rule tree (since it is already known that no intermediate 
context will need that particular generic font).

Number 2) is more difficult, but that's the way to go in order to achieve a 
better efficiency in speed and space: only one nsFont will be used in 
nsStyleFont, and the related computation will be done on demand.

Unfortunately, that's the kind of special things for which the Style System 
wasn't designed for. So I would need to special-case a code-path for this bug
in order to achieve correctness without impacting efficiency.

Apparently IE5 on the Mac has already solved the problem. I wonder how 
clean/efficient the implementation is over there. 
http://www.chasms3.com/macie5/mie5pref3.htm
OS: Windows 2000 → All
Hardware: PC → All

    
    

    
  
It just occurred to me that what IE5 does is much simpler. It assumes that the 
font-sizes (and presumably the other hard-coded properties) are the _same_. 
Under this assumption, just overriding the font name anywhere during style 
resolution does the trick. So their implementation is not really rich. Whereas 
this bug, if fixed properly, aims at offering the ability to specify different 
font-sizes (and theoretically other properties too) -- there is no reliance on 
the fact that the font-sizes and the other hard-coded values in the presentation 
context have to be the same.

    
    

    
  
[Reproducing some relevant comments that I added to bug 61883]

About the min-size: 

Normally the font-size is computed hop-by-hop. So if the hierarchy of elements 
is: grand-parent:font-size=12pt -> parent:font-size=50% -> child:font-size=200%, 
then the font-size of 'parent' is 6pt, and the font-size of 'child' is 12pt.

What happens if min-size is set to 7pt?

There are two ways to deal with this min-size:
1) enforce the min-size as soon as the current value goes below the threshold, 
and cascade with the min-size from there.
2) enforce the min-size on the terminal value.

With 1) the font-size of 'parent' will be forced to 7pt, and cascading with that
value will give the size of 'child' as 14pt.

With 2) the font-size of 'parent' will be forced to 7pt, but the cascade will 
continue with its computed size (6pt), yielding 12pt in the 'child'. So this 
approach requires caching all sizes, and knowing which one to return in 
particular situations (e.g., what should be returned getComputedStyle()?) This
is bit tricky to implement, but doable at extra effort :-)

    
    

    
  

      
      
      
      

        Attached file
          
        patch in layout v2
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        patch in content v2
        — 
      

    


  

    
    

    
  

      
      
      
      

        Attached file
          
        patch in gfxwin v2
        — 
      

    


  

    
    

    
  
Ready to receive comments/reviews. 

Here is a summary of what I did:

- I placed the bulk of the computations that were happening in ComputeFontData()
in a helper function called SetFont(), in a the same spirit as SetCoord() and 
SetColor(). Given a parent font, SetFont() computes the delta that is needed for 
the next font in the hierarchy. ComputeFontData() calls this function for normal 
processing (i.e., no generic font encountered), thus retaining the existing 
behavior.

- I added a new function called EnsureGenericFont(). This is where backtracking 
is done. Once the higher generic context from where to inherit is found, the 
function repeatedly walks the style rules to compute the CSSFont data for the 
current step and calls SetFont() to compute the delta needed for that step,
without caching the intermediate data (since it is known that nobody needs 
that).

- The overall scheme is lazy. If no generic font is encountered, then nothing 
happens. Moreover, there is a futher optimization in that if the direct parent 
is a generic context, then the current CSSFont data is simply re-used, with a 
direct call to SetFont() to step in the style context hierarchy.

- I also added support for a cross platform minimum font-size. To do this, I had 
to add a new field called mSize in nsStyleFont. This is the natural cascading 
size with which CSS computations are done, whereas mFont.size is the constrained 
size exposed for display purposes. Because this is done at that level, special 
care can be taken to exempt the chrome from the constraints. So the hacks used 
in GfxGTK (which affects the chrome, BTW) can be removed.

- The changes in layout and gfxwin provide the necessary hooks to glue things 
together. Other comments about the general principles of what is happening in 
gfxwin can be found in bug 61883.

I am seeking reviews and suggestions for improvements, especially regarding
the couple of XXX comments that I marked in nsRuleNode.cpp. Targetting to 
mozilla0.9.3.
Status: NEW → ASSIGNED
Target Milestone: --- → mozilla0.9.3

    
  
Blocks: 30910

    
    

    
  
Just noted that the dual font-sizes that I added out of necessity are the 
same ingredients needed for the font-size-adjust property (see hixie's
comments on bug 74186):
   http://www.w3.org/TR/REC-CSS2/fonts.html#propdef-font-size-adjust
   http://www.w3.org/TR/REC-CSS2/fonts.html#font-descriptions
   http://www.w3.org/TR/REC-CSS2/fonts.html#descdef-x-height

Using the CSS terminology, nsStyleFont::mFont.size is the so-called 
'actual value', while nsStyleFont::mSize is the 'computed value'.
http://www.w3.org/TR/REC-CSS2/cascade.html#q1



    
    

    
  
I attached a file that shows the full body of the SetFont() function that I 
added, together with the new outlook of ComputeFontData() when using this 
function. I also further iterated in EnsureFontData() to add some optimizations 
and make things event more elegant. The previous optimization was based on the 
observation that if the direct parent is a generic context, then the current 
CSSFont data is up-to-date and can simply be re-used in an immediate call to 
SetFont().

But it can further be observed that this same reasoning applies for the _last_ 
step of the re-application of the style rules, i.e., the current CSSFont data 
can also be re-used whithout having to re-calculate it at the final step. So I 
shuffled things around in EnsureGenericFont() to combine the two cases, with 
zero intermediate steps if the direct parent is a generic context. This led to a 
more elegant, yet optimized code. Since I dread the block & table regression 
tests, I am still beating around the bush :-)

    
    

    
  


  

    
    

    
  


  

    
    

    
  
Passed the regression tests!

The patches provide the insfrastructure that will help to solve the related 
font bugs for which this bug was intended:
- support of the basic CSS generic fonts (bug 61883)
- and some nifty accessibility issues like:
  * XP support of a pref for a minimum font-size in the client area (bug 30910).
    (this is a most wanted feature for *nix people).
  * auto adjust of generic font-sizes (cf. attachments in bug 74186)

Performance-wise, following the unification in nsStyleFont that led to the 
elimination of one nsFont (the fixed font), bug 74771 can benefit from this 
patch too.

(Other related bugs like 79074 may possibly be fixed through these changes.
If not, it shouldn't be hard now to roll them in at some stage.)

What remains to be done in the back-end for this bug is mostly confined in GFX 
now, to make it recognize the new set of prefs:
font.default.[langGroup].[generic].name = fontname1, fontname2, ... [pre-built]
font.current.[langGroup].[generic].name = user's choice
font.current.[langGroup].[generic].size = Auto | integer - user's choice
font.current.[langGroup].min-size = integer - user's choice

Gerv & mpt will be doing the XUL for the front-end. This is being covered
in bug 61883.

I would like to get clearance (r/sr/a) from you folks for this part so that I 
could try to make progress on the other part, and be ready with the whole lot
in due time for m0.9.3.
Blocks: 74771

    
    

    
  
I'm a bit concerned that allowing different default font sizes for the generic
fonts goes against the spec.  How are you doing this without breaking
inheritance?  Which approach did you choose (it's not clear from above after a
quick read)?

Also, how would a prefs UI allow the user to select font lists?

    
    

    
  
Re: different font-sizes, you might perhaps find the discussion in bug 28899
very useful in that respect. The basic argument goes that your prefrerred serif 
font can be relatively larger to your preferred sans-serif font for example. 
[C.f. also bug 74186 which has some illustrative screenshots of the problem,
together with a demo of how the 'Auto' font-size will operate; the spec has a 
'font-size-adjust' property (not-yet-implemented) which could have altered the 
font-size in a spec-compliant manner.]

Since having that working in my tree, I am finding this ability very helpful.
For example, if my default monospace font is large (or tiny), then buttons 
(or other elements with 'font-family: monospace') look ugly in pages. Just 
changing the font-size of monospace allows to fix the problem. The problem 
wouldn't have an easy solution if the font-size had to be the same, meaning I 
would have to forget using that font as my default monospace font.

Re: inheritance, what I did is equivalent to having all the nsFonts at the start 
of the style resolution process and applying the same cascading rules on them. 
So conceptually, it is like having multiple style resolution threads in which 
only the starting values are different: one thread for serif, another thread for 
sans-serif font, etc. However, since only one nsFont can be used at any one 
time, elements will pick the value computed by the thread relevant to the them. 
E.g., if my current generic font-family is serif, then I get what is computed by 
the serif thread. Since the usual cascading process is going on in each on these 
'threads' (just that the starting values are different), computed values get 
overwritten/inherited in the normal CSS sense. [This isn't how it is coded, 
but the implementation is pretty much equivalent to the above description.]

re: selection of font lists, bug 28899 had a proposal for this (using the 
principle of 'Add/Remove' on two panels -- similar to the selection of sidebar 
tabs for example). However, it was determined in that bug that doing so would 
make things too complex, and the feature wasn't implemented. The ongoing plan is 
to allow the user to select a '.current.' font for each generic font family, and 
have good factory pre-built fallback lists of '.default.' fonts for each 
language group on each platform. This is what will be happening in bug 61883.

    
    

    
  
On further thoughts, I would like to retract my patch despite the energy and 
time I put into it. Now that I have become aware of this 'font-size-adjust' 
property, it provides a spec-compliant means to achieve the same thing. It might 
be best to direct effort at implementing it.

    
    

    
  


  

    
    

    
  
Any idea how to get rid of the default fixed font now using this feature in 
order to reduce the size of nsStyleFont?  What the default fixed font is 
presently doing amounts to a cascading 'thread' that started with its own 
initial font-size.

    
    

    
  
See also the discussion in bug 74186 regarding 'font-size-adjust'.
Keywords: css2, 
          
            fonts, 
          
            qawanted
Whiteboard: [Hixie-PF]

    
    

    
  
Ok, call me dumb, but I've read and understood the relevant portions of the CSS 
spec, and I can't see how allowing the user to set different `medium' sizes for 
each CSS family is incompatible with implementing font-size-adjust. (I 
discussed this with Daniel Glazman and he couldn't see the problem either.) 
Certainly if the latter is implemented *instead of* the former, I foresee two 
classes of complaint:
(1) some unexpected and hard-to-fix phenomenon where x-height for fonts aren't
    reported reliably by a particular OS/font server/whatever, and some fonts
    end up ridiculously small/large;
(2) people who can set their prefs to get proportional text to the right size
    when reading HTML, and monospace text the right size when reading
    plain-text e-mail, but find it infuriatingly impossible to do both at the
    same time.

Opera (yada yada yada Håkon Wium Lie yada) allows different size settings for 
each of the CSS families (as well as for its various other fonts). Does that 
make it impossible for Opera to support font-size-adjust?

    
    

    
  
The _UI_ can suggest whatever it wants, so long as we end up converting it into
a font-size-adjust thing on the back end.

And Opera, despite having Håkon Lie on its payroll, is not the most standards
compliant browser around.

    
    

    
  
The conversion to font-size-adjust won't produce quite the same effect.
For example:

<p style="font-family:serif"> 
  serif 1
  <span style="font-family:verdana">
    verdana 1
    <span style="font-family:serif"> serif 2</span>
    verdana 2
  </span>
</span>

The patch had the effect of sneaking through the elements and just changing
the size of the serif texts... It would take a lot of hypothetical 
"font-size-adjust:none" (that authors won't use) to achieve the same effect.

[But I am not sure how this kind of sneaking can work with font-size-adjust at 
the same time - the behavior might be undefined]

    
    

    
  
Eek, well you wouldn't want that, that's for sure.

    
    

    
  
Which way?  Notice that -moz-fixed is doing the sneaking right now.

    
    

    
  
Errata... the sneaking is only "first-time", the first generic font gets the 
initial value and the CSS inheritance & cascade take over after that, and any 
explicit setting of the font-size takes precedence. (This errata applies to my 
comments about -moz-fixed to.) So the behavior is a little bit like -moz-initial
in quirks, with the distinctive difference that it sets the font-size only the 
first-time, so long as it hasn't yet been explicitly specified (i.e., 
overwritten by an earlier CSS cascade).

The patch will do:
<p> 
  default 'proportional (variable)' size
  <span style="font-family:-moz-fixed"> 
     default specified size for moz-fixed enters the scene
     <span style="serif">
        default specified size for serif enters the scene on this subtree
     </span
     <span style="font-family:verdana">
        verdana text which has inherited the size of -moz-fixed
        <span style="font-family:serif">
           default specified size for serif enters the scene on this subtree
        </span>
     </span>
   </span>
</p>

From this, yes a conversion to font-size-adjust in the background can produce a 
closely related effect -- it still won't be exactly the same effect because 
adjusted font-sizes depend on the actual metrics of the fonts. Indeed a generic 
size that would have cascaded will now be re-adjusted depending on the actual 
xheight of the font specified by the element.

Font-size-adjust will do:
...
  <span style="font-family:serif; font-size-adjust:0.55">
     serif text with default adjustment
     <span style="font-family:times"> 
        times text at an adjusted size s1
     </span>
     <span style="font-family:arial>
        arial text at an adjusted size s2
     </span>
  </span>
...

    
    

    
  
I have setup a branch with the complete set of changes needed for the problem 
(support for 'font-size-adjust', dynamic font heights in layout, etc) -- see 
bug 74186 for information about checking out the branch.

    
    

    
  
> Notice that -moz-fixed is doing the sneaking right now.

I've never exactly been supportive of '-moz-fixed'...

    
    

    
  
> Matthew Thomas (mpt) 2001-07-03 07:57 -------
[...]
>Certainly if the latter is implemented *instead of* the former, I foresee two 
>classes of complaint:

BTW, after being done with the proposed spec compliant way, it would take a
oneliner to re-enact different font-sizes the former way if folks find it
worthwhile enough (there could be a hidden pref or something). See the 'if'
clause with the mention to 'bug 84398' in nsPresContext::GetFontPreferences() in
the enclosed patch. One could flip back and forth from one method to another
by relaxing that 'if' clause.
http://bonsai.mozilla.org/cvsview2.cgi?diff_mode=context&whitespace_mode=show&subdir=mozilla/layout/base/src&command=DIFF_FRAMESET&file=nsPresContext.cpp&rev1=3.157&rev2=3.157.2.1&root=/cvsroot

    
    

    
  
Milestone shift... Need interested folks to try out the branch that has the fix.
It may be too late for changes like this soon. See bug 74186.
Target Milestone: mozilla0.9.3 → mozilla0.9.4

    
    

    
  
Another Milestone shift... Hopefully should be over soon.
Target Milestone: mozilla0.9.4 → mozilla0.9.5

    
    

    
  
Fixed with my font changes in bug 99010.
Status: ASSIGNED → RESOLVED
Closed: 23 years ago
Resolution: --- → FIXED
Keywords: qawanted

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






  

    
  







  

    

      
Attachment

      

      
      
      
      
    

    

      

        

          

            
General

            

              Creator: 



            

            
Created: 

            
Updated: 

            
Size: