Community
Participate
Working Groups
Created attachment 79751 [details] Patch to fix this problem based on HEAD Build ID: 3.4 Steps To Reproduce: 1. Create an RCP application that enables preference pages 2. Create a new theme extension 3. Set that theme as the current theme 4. Run the RCP app and open the preference pages. Bug: Note that the banner area in the preference page is not the proper font. On windows it's MS Sans Serif and not bold. The default is defined in JFaceResources as Tahoma 10pt bold and since the new theme does not override that font the font should default to the definition in JFaceResources. Attaching patch..
Kim, could you please review the patch? Matt can commit but would like to have someone double-check his fix.
Fix in HEAD. Thanks Matthew!
I believe I've verified this in I20071029-0010 but I'd appreciate a second look Matthew.
Looks good to me.
This fix is not OK and caused bug 224723. Should reopen, revert and find better fix.
Reopening.
I think this fix itself is fine - the original code, pre fix, was bogus. This fix has merely exposed a problem with how we handle high contrast in JFace. Either we should disregard the provided property files or we should be smarter about how we apply them. One possible solution: instead of taking them as-supplied we should take the font face and combine it with the default system font size. Alternatively, in high contrast we simply make every font default to the system font.
Kim and I discussed and I'll work on a patch
Created attachment 97026 [details] Patch to keep new behaviour but revert to system fonts in HC I've created a new patch. It simply keeps the new behaviour that Matt provided, except in HC mode where it reverts to the old behaviour (which resolves to the system font). Its a hack but the least disruptive change given the milestone. I've tested this new patch and it results in the correct text font in HC. Matt, can you verify it fixes your case too? I've create bug #228207 as a placeholder for a better fix in 3.5.
(In reply to comment #9) > Created an attachment (id=97026) [details] > Patch to keep new behaviour but revert to system fonts in HC > > I've created a new patch. It simply keeps the new behaviour that Matt > provided, except in HC mode where it reverts to the old behaviour (which > resolves to the system font). Its a hack but the least disruptive change given > the milestone. > > I've tested this new patch and it results in the correct text font in HC. > > Matt, can you verify it fixes your case too? > > I've create bug #228207 as a placeholder for a better fix in 3.5. > Instead of passing the key in could we not pass in JFaceResources.DEFAULT_FONT? This would have the same effect without the side effects that this bug corrected, namely: polluting the JFaceRegistry with keys for each of the fonts requested on a particular theme and the creation and destruction of fonts (all identical) for each requested font?
(In reply to comment #10) > Instead of passing the key in could we not pass in JFaceResources.DEFAULT_FONT? > This would have the same effect without the side effects that this bug > corrected, namely: polluting the JFaceRegistry with keys for each of the fonts > requested on a particular theme and the creation and destruction of fonts (all > identical) for each requested font? Its a good point. I was going for smallest code delta given the milestone and the complexity of jface vs. themes. My first reaction to your suggestion was that it duplicates the behaviour of how we end up with DEFAULT_FONT (right now its set where, via FontRegistry.defaultFontRecord()? ). But I guess the claim is that the HC theme is in fact wrong wrt. fonts: it's use of empty <fontOverride>'s was relying on incorrect defaulting behaviour. Our claim is: HC + empty defaultFont = DEFAULT_FONT So I think your suggestion is the right one. As a bonus, it'll be a bit easier to figure out how you end up with DEFAULT_FONT when in HC; presently its rather magical. Even some other mythical HC theme would want to default like this so there's no possibility of regression by moving the behaviour here.
Created attachment 97481 [details] Updated patch to directly use DEFAULT_FONT in HC where no default
Raji, Matt suggested you may have time to verify this. Since its a pretty straightforward delta from what he submitted I am going to go ahead and release it. Verified we get the right size text font in HC.
patch released
Verified in I20080429-0100 that we are getting the correct text font in HC. Reporter will need to say whether it fixes their case.
