According to the discussion you linked to, "Swing is trying to paint the header
you've already removed from the table..." which is why the table is null. This
is a good example of my original point. Why is Swing trying to paint a header
that has been removed?
In any case, whenever it's not obvious why a seemingly vital parameter could be
null, it would be useful if there were some comments in the code to explain
this. I have long felt that the many java APIs could be improved by specifying
which parameters are allowed to be null.
-- Miguel
On Friday, February 19, 2016 1:04 AM, Ajit Ghaisas
<[email protected]> wrote:
#yiv6463762851 #yiv6463762851 -- _filtered #yiv6463762851
{font-family:Helvetica;panose-1:2 11 6 4 2 2 2 2 2 4;} _filtered #yiv6463762851
{panose-1:2 4 5 3 5 4 6 3 2 4;} _filtered #yiv6463762851
{font-family:Calibri;panose-1:2 15 5 2 2 2 4 3 2 4;}#yiv6463762851
#yiv6463762851 p.yiv6463762851MsoNormal, #yiv6463762851
li.yiv6463762851MsoNormal, #yiv6463762851 div.yiv6463762851MsoNormal
{margin:0in;margin-bottom:.0001pt;font-size:12.0pt;}#yiv6463762851 a:link,
#yiv6463762851 span.yiv6463762851MsoHyperlink
{color:blue;text-decoration:underline;}#yiv6463762851 a:visited, #yiv6463762851
span.yiv6463762851MsoHyperlinkFollowed
{color:purple;text-decoration:underline;}#yiv6463762851
span.yiv6463762851EmailStyle17 {color:#1F497D;}#yiv6463762851
.yiv6463762851MsoChpDefault {font-size:10.0pt;} _filtered #yiv6463762851
{margin:1.0in 1.0in 1.0in 1.0in;}#yiv6463762851 div.yiv6463762851WordSection1
{}#yiv6463762851 Hi, There are the some good points made by Miguel
regarding null checks – but, in this case the table being null is a valid case.
While rendering the TableHeader, it is possible to be in a situation where
‘table’ parameter can be null. Please refer to a similar discussion :
https://community.oracle.com/thread/2279601?start=0&tstart=0 I have
done corrections to address Sergey’s second question and simplified the test
code. Please review updated webrev :
http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.04/ Regards,Ajit
From: Miguel Munoz [mailto:[email protected]]
Sent: Thursday, February 18, 2016 4:29 PM
To: Ajit Ghaisas; Sergey Bylokhov; Rajeev Chamyal; Alexander Scherbatiy;
[email protected]
Subject: Re: <Swing Dev> [9] Review fix for JDK-8020039 : SynthTableHeaderUI
refers to possibly null parameter in cell renderer It would seem to me that if
the table is null, you have a bug in another class. Is there a valid reason for
the table to be null? I would guess not.
In my experience, many null checks are either unnecessary or only serve to hide
bugs. Steve Maguire writes about this in his book Writing Solid Code. --
Miguel Muñoz On Wednesday, February 17, 2016 10:52 PM, Ajit Ghaisas
<[email protected]> wrote: Hi,
Goods finding Sergey.
1. In file,
src/java.desktop/macosx/classes/com/apple/laf/AquaTableHeaderUI.java, call to
setText() and setBorder() should be made irrespective of whether table in null
or not. I have made this change.
2. The changes to SynthTableHeaderUI.java are correct. We cannot assume a
table property (enabled) when table is null.
A call to super. getTableCellRendererComponent() is already made from
this method which in turn calls setText() and setBorder().
Please review updated webrev :
http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.03/
Regards,
Ajit
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, February 17, 2016 8:41 PM
To: Rajeev Chamyal; Ajit Ghaisas; Alexander Scherbatiy;
[email protected]
Subject: Re: <Swing Dev> [9] Review fix for JDK-8020039 : SynthTableHeaderUI
refers to possibly null parameter in cell renderer
Hi, Hello Ajit.
I am not sure that exclusion of the code is a correct fix here. For example can
you clarify should we call setText(or other setXXX) when the table is null or
not? Another question is: should we skip the code in SynthTableHeaderUI.java or
we can assume table==null as enabled table?
On 17.02.16 14:29, Rajeev Chamyal wrote:
> Looks good to me.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Ajit Ghaisas
> *Sent:* 17 February 2016 16:51
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> [email protected]
> *Subject:* RE: <Swing Dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
> I have corrected formatting of test code and removed the
> additional System.out.printlns.
>
> Please review :
> http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.02/
>
> Regards,
>
> Ajit
>
> *From:*Ajit Ghaisas
> *Sent:* Tuesday, February 16, 2016 3:12 PM
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> [email protected] <mailto:[email protected]>
> *Subject:* Re: <Swing Dev> <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
> Thanks Rajeev for review comments.
>
> I have checked - windows LAF WindowsTableHeaderUI handles it
> correctly.
>
> I have added null check for MacOs Aqua.
>
> Also, I have added a test checking for this null pointer access.
>
> Please review --
> http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.01/
>
> Regards,
>
> Ajit
>
> *From:*Rajeev Chamyal
> *Sent:* Monday, February 15, 2016 5:39 PM
> *To:* Ajit Ghaisas; Sergey Bylokhov; Alexander Scherbatiy;
> [email protected] <mailto:[email protected]>
> *Subject:* RE: <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hello Ajit,
>
> Can you please if similar fix is required for other LAF windows ,Aqua etc.
>
> Please add a regression test case also.
>
> Regards,
>
> Rajeev Chamyal
>
> *From:*Ajit Ghaisas
> *Sent:* 15 February 2016 17:30
> *To:* Rajeev Chamyal; Sergey Bylokhov; Alexander Scherbatiy;
> [email protected] <mailto:[email protected]>
> *Subject:* <Swing-dev> [9] Review fix for JDK-8020039 :
> SynthTableHeaderUI refers to possibly null parameter in cell renderer
>
> Hi,
>
> Please review the fix for jdk9,
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8020039
>
> Webrev: http://cr.openjdk.java.net/~arapte/ajit/8020039/webrev.00/
>
> Issue :
>
> If null is passed as 'table' parameter to
> SynthTableHeaderUI::getTableCellRendererComponent() method in
>
> src/java.desktop/share/classes/javax/swing/plaf/synth/SynthTableHeader
> UI.java,
>
> there is Null Pointer Exception.
>
> Analysis :
>
> This method already has a null check for 'table' parameter for
> second access of this parameter in method.
>
> Whereas the first access of the 'table' parameter lacks this check.
>
> Fix :
>
> Added null check for the first access of 'table' parameter in
> SynthTableHeaderUI::getTableCellRendererComponent().
>
> There is no else block added as the flow continues and passes
> table to base class method using super. getTableCellRendererComponent().
>
> The passed parameter is already checked in base class method
> correctly. Hence, no change is needed in base class.
>
> Test :
>
> The fix is pretty straight forward.
>
> Executed the code snippet given in the bug description. There is
> no NPE after the fix.
>
> Regards,
>
> Ajit
>
--
Best regards, Sergey.
