While investigating bug 90205, I discovered that nsStyleFont::CalcDifference() will return NS_STYLE_HINT_REFLOW if the font's mFlags differ. This seems a bit too liberal, because at least two of the flags (NS_STYLE_FONT_[FACE|SIZE]_EXPLICIT) are write-only. (The only code that uses them is some #if 0'd form control code -- see nsFormControlHelper.cpp:626; perhaps we ought remove these flags altogether?) The last flag (NS_STYLE_FONT_USE_FIXED) simply blasts the fixed font into the normal font slot AFAICT. So... Do we care if the flags differ at all? Could we just remove the flag comparison code in CalcDifference() and let the normal font comparison routines do their thing?
Status: NEW → ASSIGNED
Priority: -- → P4
Target Milestone: --- → mozilla1.0
If the flag NS_STYLE_FONT_USE_FIXED changes, I think we need to reflow since the layout will change to use or not use the fixed font, right? Other than that, I think you are probably right about the other flags (in other words, we should remove the unused flags and restrict the flag comparison to just the USE_FIXED flag - unless that turns out to be unnecessary too).
But at nsRuleNode.cpp:1527 (where we actually fill in the nsStyleFont's data), we explicitly reset mFont to mFixedFont. (This is the only place that the NS_STYLE_FONT_USE_FIXED is read.) With that in mind, nsStyleFont::CalcFontDifference() should do the right thing, no?
Yup, you're right. Since nsStyleFont::CalcDifference() computes the difference between both fonts (fixed and non-fixed) it will correctly handle the swith made in the ruleNode.
In other words, I can get rid of mFlags from nsStyleFont in my patch at that other bug 84398, since I don't use NS_STYLE_FONT_USE_FIXED anymore. A little more savings.
Please get in touch with firstname.lastname@example.org before removing these flags.
Rod is on sabbatical - he won't be back for 5 more weeks. If he really objects, he can always add back the dead, unused and ancient code (he #if 0'd it on Jan 25, 2000).
Rod's on sabbatical? Cool! I'm going to re-enable the native widgets next week!
Changes checked in.
Status: ASSIGNED → RESOLVED
Last Resolved: 17 years ago
Resolution: --- → FIXED
Adding this here for the record. As I was about to remove mFlags from FONT_20011307_BRANCH, it occurred to me that one information carried by these flags was to differentiate between the following two situations: <span style="font-size:24pt"> ... <span style="font-size:50%"> ... text at 12pt ... </span> </span> vs: (note: '50%' above could be something else, e.g., 'smaller' with a different parent font, and there could be an hierarchy of such elements) <span style="font-size:24pt"> ... <span style="font-size:12pt"> ... text at 12pt ... </span> </span> But the direct test on the final resolved font structs would tell if there is any difference and ensure that a reflow hint is returned. So I do not see why mFlags should be kept if it is write-only. There is a subtelty as to whether the CalcLength()/CalPointSize() functions (the functions that maps the non-fixed values to the final fixed values) can lead to edge cases that warrant having the test against mFlags in CalcFontDifference() somewhere in the StyleContext hierarchy. So far, I can't picture such edge cases that would not be detected by the direct test on the final resolved font structs. So off I go, I am going to remove mFlags in FONT_20011307_BRANCH because it is write-only there.
would the crash at http://dhtmlkitchen.com/dhtml/dom/extendingDOM.html be related to this?
since the code was checked in more than a year and a half ago, marking the bug verified
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.