<Swing Dev> Reviewer needed: fix for TitledBorder class
Alexander Scherbatiy
alexandr.scherbatiy at oracle.com
Mon May 28 10:16:31 UTC 2012
On 5/14/2012 5:21 PM, Pavel Tisnovsky wrote:
> ping - can anybody please look at this issue?
The 3d version of the fix looks good for me.
Thanks,
Alexandr.
> Thank you in advance,
> Pavel
>
>
> Pavel Tisnovsky wrote:
>> Pavel Porvatov wrote:
>>> Hi Pavel,
>>>> Alexander Scherbatiy wrote:
>>>>> On 5/2/2012 5:09 PM, Pavel Tisnovsky wrote:
>>>>>> Hi Pavel and Alexander,
>>>>>> I've prepared second version of fix for TitledBorder class& JTreg
>>>>>> test is
>>>>>> included too. Both changes can be seen here:
>>>>>>
>>>>>> http://cr.openjdk.java.net/~ptisnovs/TitledBorder_v2/
>>>>>>
>>>>>> Could you please look at it?
>>>>> Just two brief questions.
>>>>> - Why do we need the extra Font f variable? Is it possible to use
>>>>> only the titleFont?
>>>> Yes, you are right - it is not really needed, it only makes debugging
>>>> easier.
>>>> It's possible to shorten both getters to one liner.
>>> I'm also voting to remove unnecessary variable.
>> Hi Pavel and Alexander,
>>
>> here's third version of TitledBorder fix - getters now don't contain
>> any temporary variable:
>>
>> http://cr.openjdk.java.net/~ptisnovs/TitledBorder_v3/
>>
>>>>> - In case if the titleFont variable is null would it be better to
>>>>> set
>>>>> it to the default L&F font?
>>>>> So in the next getTitleFont() call the
>>>>> UIManager.getFont("TitledBorder.font") method will not be executed.
>>>> Hmm it might be better (it is even possible to do it directly in
>>>> setters which
>>>> could be called from constructor), but
>>>> it changes object state (it's attributes) and it IMHO could(?) cause
>>>> problems
>>>> with serialization and/or XML encoding. If you think it's not a problem
>>>> I'll be happy to create new version of this fix.
>>> I think we can't set variable, because after LAF changing the current
>>> functionality will be broken.
>>>
>>> BTW: why don't you use Oracle copyright in test header?
>> Well, I'm not Oracle employee ;-) I've asked Mark Reinhold about this
>> issue and he told me that it's ok to use RH copyright header.
>>
>> Thank,
>> Pavel
>>> Regards, Pavel
>>>>> Thanks,
>>>>> Alexandr.
>>>>>
>>>>>> Thank you in advance,
>>>>>> Pavel
>>>>>>
>>>>>>> Regards, Pavel
>>>>>>>>> Thanks,
>>>>>>>>> Alexandr.
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> On 4/27/2012 1:14 PM, Pavel Tisnovsky wrote:
>>>>>>>>>> Hi,
>>>>>>>>>>
>>>>>>>>>> I think there's a bug in a TitledBorder class. According to JavaDoc
>>>>>>>>>> the methods getTitleColor() and getTitleFont() should use look&feel
>>>>>>>>>> settings when nothing is explicitly changed by constructor/setters:
>>>>>>>>>>
>>>>>>>>>> <javadoc>
>>>>>>>>>> If the border, font, or color property values are not specified in
>>>>>>>>>> the
>>>>>>>>>> constuctor or by invoking the appropriate set
>>>>>>>>>> methods, the property values will be defined by the current look
>>>>>>>>>> and
>>>>>>>>>> feel, using the following property names in the
>>>>>>>>>> Defaults Table:
>>>>>>>>>> "TitledBorder.border"
>>>>>>>>>> "TitledBorder.font"
>>>>>>>>>> "TitledBorder.titleColor"
>>>>>>>>>> </javadoc>
>>>>>>>>>>
>>>>>>>>>> This behaviour were removed by following changeset:
>>>>>>>>>> changeset: 2529:d062afbe2107
>>>>>>>>>> user: malenkov
>>>>>>>>>> date: Thu Jul 01 18:09:45 2010 +0400
>>>>>>>>>> summary: 4129681: Cannot get a title border to display its
>>>>>>>>>> label
>>>>>>>>>> as disabled
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Here is a webrew which contains fix for this issue:
>>>>>>>>>> http://cr.openjdk.java.net/~ptisnovs/TitledBorder/
>>>>>>>>>>
>>>>>>>>>> Can anybody please review this fix?
>>>>>>>>>> (I'd like to push the same change to OpenJDK7 too it it will be
>>>>>>>>>> reviewed& accepted)
>>>>>>>>>>
>>>>>>>>>> Thank you in advance,
>>>>>>>>>> Pavel Tisnovsky
More information about the swing-dev
mailing list