<AWT Dev> Review request: 8005932 : Java 7 on mac os x only provides text clipboard formats
Denis S. Fokin
denis.fokin at oracle.com
Fri Feb 8 08:12:53 PST 2013
Hi Mikhail,
the fix and the test look fine.
Thank you,
Denis.
On 2/8/2013 5:55 PM, mikhail cherkasov wrote:
> Denis,
>
> MacOSX -> Mac OS X:
> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.06/
> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.06/>
>
> I've run only jtreg tests, they all work fine
>
> 08.02.2013 16:58, Denis S. Fokin пишет:
>> Hi Mikhail,
>>
>> In the jdk6 file the system was "Mac OS X". I think, Apple naming is
>> preferable.
>>
>> Have you run JCK and JTreg dnd and data transfer tests?
>>
>> Thank you,
>> Denis.
>>
>> On 2/8/2013 3:50 PM, mikhail cherkasov wrote:
>>> Hello again,
>>>
>>> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.05/
>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.05/>
>>> I've replaced "X11 " with "MacOSX" and swap html and rtf order to
>>> correspond to jdk6.
>>>
>>> Thanks,
>>> Mikhail.
>>>
>>> 08.02.2013 15:12, Denis S. Fokin пишет:
>>>> Hi Mikhail,
>>>>
>>>> the test looks good. I have just noticed that we copied X11
>>>> flvormaps.properies file and did not change the text of the comments.
>>>> Mentioning of X11 on MacOSX looks confusing.
>>>>
>>>> 3 # java.awt.datatransfer.SystemFlavorMap. It contains the X11
>>>> platform-specific,
>>>> 4 # default mappings between common X11 selection atoms and
>>>> platform-independent
>>>> 5 # MIME type strings, which will be converted into
>>>>
>>>> Would you mind changing the text? Additionally, I looked at the jdk6
>>>> flavormap.propertires file and noticed that the order of html and rtf
>>>> types is different. So we have a chance that we will provide the bet
>>>> text flavor java.awt.datatransfer.DataFlavor.selectBestTextFlavor
>>>> differently comparing to the jdk6. So I would preserve the order in
>>>> the change.
>>>>
>>>> Thank you,
>>>> Denis.
>>>>
>>>>
>>>> On 2/8/2013 1:43 PM, mikhail cherkasov wrote:
>>>>> Hello there,
>>>>>
>>>>> New version, with new test:
>>>>> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.04/
>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.04/>
>>>>> There I've implemented the test with dnd between two JVM.
>>>>>
>>>>> Thanks,
>>>>> Mikhail.
>>>>>
>>>>> 05.02.2013 16:50, mikhail cherkasov пишет:
>>>>>> Hello Denis, All,
>>>>>>
>>>>>> Here is new version:
>>>>>> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.03/
>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.03/>
>>>>>> Please review it.
>>>>>>
>>>>>> 01.02.2013 19:57, Denis S. Fokin пишет:
>>>>>>> Hi Mikhail,
>>>>>>> The fix looks good.
>>>>>>> As for the test, personally, I would not implement it this way. For
>>>>>>> AWT functionality we use Swing library. By this reason we use a hack
>>>>>>> with final array.
>>>>>>> I would replace the next loop
>>>>>>> 108 for (int i = 0; i < 2; i++) {
>>>>>>> 109 DropObject.getInstance().setCurrentDf(i % 2 == 0 ?
>>>>>>> rtfDataFlavor : htmlDataFlavor);
>>>>>>> with function invocation where I would directly pass "from" and
>>>>>>> "to"
>>>>>>> points instead of swapping variables.
>>>>>>> I am not sure that I am understand the next lines
>>>>>>> 203 if (dropObject != null) {
>>>>>>> 204 dtde.rejectDrop();
>>>>>>> Are we use the transferrable as an initialization flag?
>>>>>>> Usually, Transferrable objects contain data that can be converted by
>>>>>>> demand into particular type.
>>>>>>> The DropObject always returns html as bytes (for the test purposes
>>>>>>> this is ok). On the other hand, the condition
>>>>>>> 109 DropObject.getInstance().setCurrentDf(i % 2 == 0 ?
>>>>>>> rtfDataFlavor : htmlDataFlavor);
>>>>>>> should be implemented in transferable and available flavors
>>>>>>> should be
>>>>>>> encapsulated.
>>>>>>> The data transfer part of the test should work as follows:
>>>>>>> 1. Transferable contains an array with two DataFlavor instances
>>>>>>> 2. isDataFlavorSupported returns true if the array contains the
>>>>>>> DataFlavor passed as a parameter
>>>>>>> 3. getTransferDataFlavors returns the array
>>>>>>> 4. getTransferData works as you have implemented it
>>>>>>> The transferable does too many things. It can calculate position of
>>>>>>> something, draw something on graphics and work as a transferrable.
>>>>>>> Calculation and drawing are parts of the test not transferable (what
>>>>>>> is more, we calculate position for a panel that does not have direct
>>>>>>> relation to the transferred data).
>>>>>>> On drop I would prefer to use frame disposal instead of
>>>>>>> System.exit()
>>>>>>> 67 System.exit(0);
>>>>>>> The test could be implemented with Clipboard API only. On the other
>>>>>>> hand, it could be implemented in a two different processes to verify
>>>>>>> that the data is passed between JVMs.
>>>>>>> If the current implementation works well on Windows and MacOS X
>>>>>>> platforms, I am ok with the webrev.
>>>>>>> Thank you,
>>>>>>> Denis.
>>>>>>>
>>>>>>>
>>>>>>> On Feb 1, 2013, at 5:59 PM, mikhail cherkasov wrote:
>>>>>>>
>>>>>>>> Hello Denis, All,
>>>>>>>>
>>>>>>>> There's new version
>>>>>>>> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.01/
>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.01/>
>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.01/>
>>>>>>>> I exclude solaris's changes and also I've rewritten test.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Mikhail.
>>>>>>>>
>>>>>>>> 30.01.2013 18:13, Denis S. Fokin пишет:
>>>>>>>>> Hi Mikhail,
>>>>>>>>>
>>>>>>>>> the fix for macosx looks ok. I would run jtreg and jck dnd and
>>>>>>>>> datatransfer tests with the change.
>>>>>>>>>
>>>>>>>>> Have you gotten any benefits from the solaris change?
>>>>>>>>>
>>>>>>>>> Thank you,
>>>>>>>>> Denis.
>>>>>>>>>
>>>>>>>>> On 1/29/2013 6:08 PM, mikhail cherkasov wrote:
>>>>>>>>>> Hello again,
>>>>>>>>>>
>>>>>>>>>> This bug is still waiting for review.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Mikhail.
>>>>>>>>>>
>>>>>>>>>> 25.01.2013 19:41, mikhail cherkasov пишет:
>>>>>>>>>>> Hello All,
>>>>>>>>>>>
>>>>>>>>>>> Please review the fix for the following bug:
>>>>>>>>>>> http://bugs.sun.com/view_bug.do?bug_id=8005932
>>>>>>>>>>> Fix:
>>>>>>>>>>> http://cr.openjdk.java.net/~mcherkas/8005932/webrev.00/
>>>>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.00/>
>>>>>>>>>>> <http://cr.openjdk.java.net/%7Emcherkas/8005932/webrev.00/>
>>>>>>>>>>>
>>>>>>>>>>> There's no code changes, I've just added missed mime types to
>>>>>>>>>>> macosx
>>>>>>>>>>> and linux flavormap.properties.
>>>>>>>>>>>
>>>>>>>>>>> Thanks,
>>>>>>>>>>> Mikhail.
>>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>
>>>>>>>
>>>>>>
>>>>>
>>>>
>>>
>>
>
More information about the awt-dev
mailing list