<Swing Dev> [10] RFR JDK-7042497: javax.swing.JOptionPane.showInternalConfirmDialog throws RuntimeException

[Semyon Sadetsky]

Looks good.

--Semyon


On 05/18/2017 11:07 PM, Prasanta Sadhukhan wrote:
>
> Please find the modified webrev
>
> http://cr.openjdk.java.net/~psadhukhan/7042497/webrev.04/
>
> Regards
> Prasanta
> On 5/18/2017 8:44 PM, Semyon Sadetsky wrote:
>>
>> This version of the fix formally corresponds to the spec. (I did not 
>> check the spec for validity.)
>>
>>         if (parentComponent == null)
>>             return false;
>>         if (parentComponent instanceof Frame)
>>             return true;
>>         return 
>> JOptionPane.checkFrameForComponent(parentComponent.getParent());
>>
>> - "JOptionPane." is not necessary.
>>
>> - Please, add braces to the both if conditions.
>>
>>         if (parentComponent == null || 
>> !(checkFrameForComponent(parentComponent))) {
>>
>> - "parentComponent == null" is not necessary now
>>
>> - to avoid extra "not" operation the if statement could be inverted.
>>
>> --Semyon
>>
>>
>> On 05/18/2017 12:01 AM, Prasanta Sadhukhan wrote:
>>>
>>> Not sure about the exact meaning of that but anyways tried to make 
>>> the code spec-compliant.
>>>
>>> http://cr.openjdk.java.net/~psadhukhan/7042497/webrev.03/
>>>
>>> If you think it still does not adhere to spec, please suggest what 
>>> more needs to be done.
>>>
>>> Regards
>>> Prasanta
>>> On 5/17/2017 8:23 PM, Semyon Sadetsky wrote:
>>>>
>>>> It seems, !(parentComponent instanceof Frame)) is not what the spec 
>>>> means.
>>>>
>>>> --Semyon
>>>>
>>>> On 05/17/2017 12:55 AM, Prasanta Sadhukhan wrote:
>>>>> Ok, sorry. Updated code to get the default Frame when component 
>>>>> has no Frame and
>>>>> testcase to test that case too.
>>>>> Updated webrev
>>>>> http://cr.openjdk.java.net/~psadhukhan/7042497/webrev.02/
>>>>>
>>>>> Regards
>>>>> Prasanta
>>>>> On 5/16/2017 8:47 PM, Semyon Sadetsky wrote:
>>>>>>
>>>>>> Yes, sorry. I meant the case I've mentioned in my first comment : 
>>>>>> when the component has no frame.
>>>>>>
>>>>>> parentComponent determines the <code>Frame</code> * in which the 
>>>>>> dialog is displayed; if <code>null</code>, * *_or if the _**_<code>_**_parentComponent_**_</code> _**_has no _**_* _**_<code>_**_Frame_**_</code>_**_***_*, a default <code>Frame</code> is used
>>>>>>
>>>>>> --Semyon
>>>>>>
>>>>>> On 05/15/2017 10:44 PM, Prasanta Sadhukhan wrote:
>>>>>>>
>>>>>>> Not sure which testcase you are running. I do not see this 
>>>>>>> exception when I run my testcase in the webrev.
>>>>>>>
>>>>>>> I am using createDialog in the fix for null parentComponent and 
>>>>>>> not createInternalFrame() which throws this exception.
>>>>>>>
>>>>>>> Regards
>>>>>>> Prasanta
>>>>>>> On 5/15/2017 11:04 PM, Semyon Sadetsky wrote:
>>>>>>>>
>>>>>>>> I don't see how this change fixes the issue.
>>>>>>>>
>>>>>>>> I still got
>>>>>>>>
>>>>>>>> Exception in thread "main" java.lang.RuntimeException: 
>>>>>>>> JOptionPane: parentComponent does not have a valid parent
>>>>>>>>
>>>>>>>> after applying it.
>>>>>>>>
>>>>>>>> --Semyon
>>>>>>>>
>>>>>>>>
>>>>>>>> On 05/15/2017 03:14 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>
>>>>>>>>> Updated webrev to use getRootFrame() via createDialog() to get 
>>>>>>>>> the default Frame when a frame is not provided.
>>>>>>>>>
>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/7042497/webrev.01/
>>>>>>>>>
>>>>>>>>> Regards
>>>>>>>>> Prasanta
>>>>>>>>> On 5/12/2017 8:08 PM, Semyon Sadetsky wrote:
>>>>>>>>>>
>>>>>>>>>> Also the spec says to use the default frame but the fix 
>>>>>>>>>> creates new frame each time.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 05/12/2017 07:32 AM, Semyon Sadetsky wrote:
>>>>>>>>>>>
>>>>>>>>>>> The fix covers the parentComponent=null case.
>>>>>>>>>>>
>>>>>>>>>>> What about the second case when non-null component is 
>>>>>>>>>>> provided but it has no frame?
>>>>>>>>>>>
>>>>>>>>>>> --Semyon
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> On 05/12/2017 12:51 AM, Prasanta Sadhukhan wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi All,
>>>>>>>>>>>>
>>>>>>>>>>>> Please review a spec violation issue whereby if in 
>>>>>>>>>>>> JOptionPane.showInternalConfirmDialog(Component 
>>>>>>>>>>>> parentComponent, Object message)
>>>>>>>>>>>> "parentComponent" is null, we are getting RuntimeException 
>>>>>>>>>>>> "parentComponent does not have a valid parent"
>>>>>>>>>>>>
>>>>>>>>>>>> but as per spec, 
>>>>>>>>>>>> https://docs.oracle.com/javase/8/docs/api/index.html?javax/swing/JOptionPane.html
>>>>>>>>>>>> it says "|parentComponent| - determines the |Frame| in 
>>>>>>>>>>>> which the dialog is displayed; if |null|, or if the 
>>>>>>>>>>>> |parentComponent| has no |Frame|, a default |Frame| is used"
>>>>>>>>>>>>
>>>>>>>>>>>> This violation exists for all overridden 
>>>>>>>>>>>> showInternalConfirmDialog methods and showInternalOptionDialog.
>>>>>>>>>>>>
>>>>>>>>>>>> The proposed fix creates a default JFrame and shows the 
>>>>>>>>>>>> confirm dialog in it, as stated in the spec.
>>>>>>>>>>>>
>>>>>>>>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-7042497
>>>>>>>>>>>> webrev: 
>>>>>>>>>>>> http://cr.openjdk.java.net/~psadhukhan/7042497/webrev.00/
>>>>>>>>>>>>
>>>>>>>>>>>> Regards
>>>>>>>>>>>> Prasanta 
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/swing-dev/attachments/20170519/56e620b1/attachment.html>