<AWT Dev> [8] Review request JDK-8160941: [macosx]"text/uri-list" dataflavor concats the first two strings

Robin Stevens stevensro at gmail.com
Mon Jul 11 15:55:10 UTC 2016


Hello Mikhail,

swapping the order of the statements in the loop was my first idea as well.
However, that would cause an inconsistency with other operating systems.
When I try with JDK8 on a Linux (Ubuntu) machine, I always get a string
with a line separator at the end:

Drag-and-drop a single file: transferable data contains the file path
followed by a line separator
Drag-and-drop multiple files: transferable data contains all the file
paths, where at the end of each file path there is a line separator
(including for the last file path).

As such, I think my fix is correct. If I apply your suggestion, the
behavior on Mac would be different from the behavior on Linux.

Note that the previous version of the code on Mac would just "forget" the
line break between the first two paths. But if you drag-and-dropped 3
files, there would be a line separator behind file1+file2, and a line
separator behind file3.
My suggested fix simply adds the missing separator between file1 and file2,
and leaves the rest of the behavior the same.

Do you still think your remark is valid, and I need to adjust my proposed
patch ?
Or can I leave it like it is ?

Robin

On Mon, Jul 11, 2016 at 3:06 PM, Mikhail Cherkasov <
mikhail.cherkasov at oracle.com> wrote:

> Hi Robin,
>
> in your fix there will be extra separator at the end of the text,
> you need to put "sb.append(separator);" before "sb.append(strings[i]);"
> in for cycle.
>
> Thanks,
> Mikhail.
>
>
> On 08.07.2016 23:06, Robin Stevens wrote:
>
> Please review the fix for JDK-8160941 .
>
> Webrev: http://cr.openjdk.java.net/~rstevens/8160941/webrev/
>
> The problem: when copy-pasting (or drag-and-dropping) multiple files, the
> data in the transferable for the flavor "text/uri-list" concats the first
> two paths.
>
> If you for example copy:
> /Users/robin/Desktop/file1.txt
> /Users/robin/Desktop/file2.txt
> /Users/robin/Desktop/file3.txt
>
> the data in the transferable is
>
> /Users/robin/Desktop/file1.txt/Users/robin/Desktop/file2.txt
> /Users/robin/Desktop/file3.txt
>
> while the expected data is
>
> /Users/robin/Desktop/file1.txt
> /Users/robin/Desktop/file2.txt
> /Users/robin/Desktop/file3.txt
>
>
> This is also what you can observe when running the manual testcase.
> As there was already a manual testcase available (which fails on JDK8),
> the patch does not include one.
>
> Thanks,
>
> Robin
>
> On Fri, Jul 8, 2016 at 9:53 AM, Alexandr Scherbatiy <
> <alexandr.scherbatiy at oracle.com>alexandr.scherbatiy at oracle.com> wrote:
>
>> On 7/7/2016 10:07 PM, Robin Stevens wrote:
>>
>> Thanks Alexander for creating the issue in the bug tracker and hosting
>> the webrev.
>> Do I need to send a new review request to the list with the official bug
>> number in the subject, or is the current email thread sufficient ?
>>
>>
>>   Yes. You can just reply on this email, correct the bug id and provide
>> the webrev link.
>>
>>   Thanks,
>>   Alexandr.
>>
>>
>> Robin
>>
>> On Thu, Jul 7, 2016 at 9:36 AM, Alexandr Scherbatiy <
>> alexandr.scherbatiy at oracle.com> wrote:
>>
>>>
>>>   The issue is recorded under id  JDK-8160941 "text/uri-list" dataflavor
>>> concats the first two strings
>>>     https://bugs.openjdk.java.net/browse/JDK-8160941
>>>
>>>   The webrev is uploaded to
>>> <http://cr.openjdk.java.net/%7Ealexsch/robin.stevens/8160941/webrev.00>
>>> http://cr.openjdk.java.net/~alexsch/robin.stevens/8160941/webrev.00
>>>
>>>   Thanks,
>>>   Alexandr.
>>>
>>> On 7/7/2016 10:10 AM, Robin Stevens wrote:
>>>
>>> Hello,
>>>
>>> the backport of the fix for
>>> <https://bugs.openjdk.java.net/browse/JDK-8136763>
>>> https://bugs.openjdk.java.net/browse/JDK-8136763 looks incorrect.
>>> The corresponding manual test case fails on jdk8.
>>>
>>> The problem: when copy-pasting (or drag-and-dropping) multiple files,
>>> the data in the transferable for the flavor "text/uri-list" concats the
>>> first two paths.
>>>
>>> If you for example copy:
>>> /Users/robin/Desktop/file1.txt
>>> /Users/robin/Desktop/file2.txt
>>> /Users/robin/Desktop/file3.txt
>>>
>>> the data in the transferable is
>>>
>>> /Users/robin/Desktop/file1.txt/Users/robin/Desktop/file2.txt
>>> /Users/robin/Desktop/file3.txt
>>>
>>> while the expected data is
>>>
>>> /Users/robin/Desktop/file1.txt
>>> /Users/robin/Desktop/file2.txt
>>> /Users/robin/Desktop/file3.txt
>>>
>>>
>>> This is also what you can observe when running the manual testcase.
>>>
>>> I have logged this in the bug database as issue JI-9041413.
>>>
>>> Attached you find a webrev with the proposed patch.
>>> The manual testcase succeeds with this patch, and fails without.
>>> At the bottom of this email, you also find the output of hg diff.
>>>
>>> The patch does not include a testcase, as there is already one available.
>>>
>>> Regards,
>>>
>>> Robin
>>>
>>>
>>> hg diff output:
>>>
>>> diff -r 0844fa517c35
>>> src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java
>>> --- a/src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java Tue Jul
>>> 05 11:03:13 2016 -0700
>>> +++ b/src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java Thu Jul
>>> 07 08:48:37 2016 +0200
>>> @@ -148,6 +148,7 @@
>>>              StringBuilder sb = new StringBuilder();
>>>              if(strings.length > 0) {
>>>                  sb.append(strings[0]);
>>> +                sb.append(separator);
>>>                  for(int i = 1; i < strings.length; i++) {
>>>                      sb.append(strings[i]);
>>>                      sb.append(separator);
>>>
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160711/0e3b17a0/attachment.html>


More information about the awt-dev mailing list