<Swing Dev> Reviewer needed: fix for TitledBorder class
Pavel Tisnovsky
ptisnovs at redhat.com
Mon May 28 11:02:50 UTC 2012
Alexander Scherbatiy wrote:
> 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.
That's great, TY Alexander.
>
>> 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