[8u-dev] Request for Approval: 8160941: [macosx]"text/uri-list" dataflavor concats the first two strings

[Claes Redestad]

Hi,

consistency seems a legitimate reason to keep this as is; reading the 
code without this context it seems calling it out with a comment would 
be appropriate.

/Claes

On 2016-07-15 15:46, Robin Stevens wrote:
> Hi,
>
> this was discussed in the previous review thread as well 
> (http://mail.openjdk.java.net/pipermail/awt-dev/2016-July/011642.html).
> It will indeed add a separator at the end (a line separator), but that 
> is the case on Linux as well.
> Drag-and-dropping a single file results in data which is the path to 
> the file + a separator.
>
> You can see this in 
> http://hg.openjdk.java.net/jdk8u/jdk8u/jdk/file/37b61c31e766/src/share/classes/sun/awt/datatransfer/DataTransferer.java 
> (which I think where the contents of the transferable on Linux is 
> created).
> Line 1391-1395:
>
>                                 if (0 != allowedFiles.length())
>                                 {
> allowedFiles.append("\\r\\n");
>                                 }
>
> So even for a single file, the separator is appended.
>
> Note that because you do not know in advance whether the transferable 
> contains multiple files or not, typical handling code is something like:
>
>     BufferedReader r = new BufferedReader(new StringReader((String) 
> aTransferable.getTransferData(URI_LIST_FLAVOR)));
>     String line = null;
>     while ((line = r.readLine()) != null) {
>         //do something with the line
>     }
>
>
> So while I could get rid of the separator for a single entry, I do not 
> thing it is needed. And for consistency among different OS, I think it 
> would even be better to leave the separator in there.
>
> Robin
>
> On Fri, Jul 15, 2016 at 2:25 PM, Claes Redestad 
> <claes.redestad at oracle.com <mailto:claes.redestad at oracle.com>> wrote:
>
>     Hi,
>
>     this seems wrong as it will add a comma at the end if there's only
>     one item.
>
>     Shouldn't this rather be something like:
>
>     sb.append(strings[0]);
>     for(int i = 1; i < strings.length; i++) {
>     +                    sb.append(separator);
>                          sb.append(strings[i]);
>     -                    sb.append(separator);
>                      }
>
>     Thanks!
>
>     /Claes
>
>
>     On 2016-07-15 14:12, Mikhail Cherkasov wrote:
>
>         Hi all,
>
>         Could you please approve integration of the following fix:
>
>         webrev:
>         http://cr.openjdk.java.net/~rstevens/8160941/webrev.01/
>         <http://cr.openjdk.java.net/%7Erstevens/8160941/webrev.01/>
>         jbs: https://bugs.openjdk.java.net/browse/JDK-8160941
>         review:
>         http://mail.openjdk.java.net/pipermail/awt-dev/2016-July/011642.html
>
>         Thanks,
>         Mikhail.
>
>
>