<div dir="ltr"><div><div><div><div><div><div><div><div><div>Hello Mikhail,<br><br></div>swapping the order of the statements in the loop was my first idea as well.<br>However, that would cause an inconsistency with other operating systems.<br></div>When I try with JDK8 on a Linux (Ubuntu) machine, I always get a string with a line separator at the end:<br><br></div>Drag-and-drop a single file: transferable data contains the file path followed by a line separator<br></div>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).<br><br></div>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.<br></div><br>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.<br></div>My suggested fix simply adds the missing separator between file1 and file2, and leaves the rest of the behavior the same.<br><br></div>Do you still think your remark is valid, and I need to adjust my proposed patch ?<br></div><div>Or can I leave it like it is ?<br></div><div><br></div>Robin<br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Jul 11, 2016 at 3:06 PM, Mikhail Cherkasov <span dir="ltr"><<a href="mailto:mikhail.cherkasov@oracle.com" target="_blank">mikhail.cherkasov@oracle.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
  
    
  
  <div bgcolor="#FFFFFF" text="#000000">
    Hi Robin,<br>
    <br>
    in your fix there will be extra separator at the end of the text,<br>
    you need to put "sb.append(separator);" before
    "sb.append(strings[i]);"<br>
    in for cycle.<br>
    <br>
    Thanks,<br>
    Mikhail.<div><div class="h5"><br>
    <br>
    <div>On 08.07.2016 23:06, Robin Stevens
      wrote:<br>
    </div>
    <blockquote type="cite">
      <div dir="ltr">Please review the fix for JDK-8160941 .
        <div><br>
        </div>
        <div>Webrev: <a href="http://cr.openjdk.java.net/%7Erstevens/8160941/webrev/" target="_blank">http://cr.openjdk.java.net/~rstevens/8160941/webrev/</a></div>
        <div><br>
        </div>
        <div>
          <div>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.</div>
          <div><br>
          </div>
          <div>If you for example copy:</div>
          <div>/Users/robin/Desktop/file1.txt</div>
          <div>/Users/robin/Desktop/file2.txt</div>
          <div>/Users/robin/Desktop/file3.txt</div>
          <div><br>
          </div>
          <div>the data in the transferable is</div>
          <div><br>
          </div>
          <div>/Users/robin/Desktop/file1.txt/Users/robin/Desktop/file2.txt</div>
          <div>/Users/robin/Desktop/file3.txt</div>
          <div><br>
          </div>
          <div>while the expected data is</div>
          <div><br>
          </div>
          <div>/Users/robin/Desktop/file1.txt</div>
          <div>/Users/robin/Desktop/file2.txt</div>
          <div>/Users/robin/Desktop/file3.txt</div>
          <div><br>
          </div>
          <div><br>
          </div>
          <div>This is also what you can observe when running the manual
            testcase.</div>
        </div>
        <div>As there was already a manual testcase available (which
          fails on JDK8), the patch does not include one.</div>
        <div><br>
        </div>
        <div>Thanks,</div>
        <div><br>
        </div>
        <div>Robin<br>
          <div class="gmail_extra"><br>
            <div class="gmail_quote">On Fri, Jul 8, 2016 at 9:53 AM,
              Alexandr Scherbatiy <span dir="ltr"><<a href="mailto:alexandr.scherbatiy@oracle.com" target="_blank"></a><a href="mailto:alexandr.scherbatiy@oracle.com" target="_blank">alexandr.scherbatiy@oracle.com</a>></span>
              wrote:<br>
              <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
                <div bgcolor="#FFFFFF" text="#000000"><span> On
                    7/7/2016 10:07 PM, Robin Stevens wrote:<br>
                    <blockquote type="cite">
                      <div dir="ltr">Thanks Alexander for creating the
                        issue in the bug tracker and hosting the webrev.
                        <div>
                          <div>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 ?</div>
                        </div>
                      </div>
                    </blockquote>
                    <br>
                  </span>   Yes. You can just reply on this email,
                  correct the bug id and provide the webrev link.<br>
                  <br>
                    Thanks,<br>
                    Alexandr.
                  <div>
                    <div><br>
                      <blockquote type="cite">
                        <div dir="ltr">
                          <div>
                            <div><br>
                            </div>
                            <div>Robin</div>
                          </div>
                        </div>
                        <div class="gmail_extra"><br>
                          <div class="gmail_quote">On Thu, Jul 7, 2016
                            at 9:36 AM, Alexandr Scherbatiy <span dir="ltr"><<a href="mailto:alexandr.scherbatiy@oracle.com" target="_blank">alexandr.scherbatiy@oracle.com</a>></span>
                            wrote:<br>
                            <blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">
                              <div bgcolor="#FFFFFF" text="#000000"> <br>
                                  The issue is recorded under id 
                                JDK-8160941 "text/uri-list" dataflavor
                                concats the first two strings<br>
                                    <a href="https://bugs.openjdk.java.net/browse/JDK-8160941" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8160941</a><br>
                                <br>
                                  The webrev is uploaded to <a href="http://cr.openjdk.java.net/%7Ealexsch/robin.stevens/8160941/webrev.00" target="_blank"></a><a href="http://cr.openjdk.java.net/~alexsch/robin.stevens/8160941/webrev.00" target="_blank">http://cr.openjdk.java.net/~alexsch/robin.stevens/8160941/webrev.00</a><br>
                                <br>
                                  Thanks,<br>
                                  Alexandr.<br>
                                <br>
                                <div>On 7/7/2016 10:10 AM, Robin Stevens
                                  wrote:<br>
                                </div>
                                <blockquote type="cite">
                                  <div dir="ltr">Hello,
                                    <div><br>
                                    </div>
                                    <div>the backport of the fix for <a href="https://bugs.openjdk.java.net/browse/JDK-8136763" target="_blank"></a><a href="https://bugs.openjdk.java.net/browse/JDK-8136763" target="_blank">https://bugs.openjdk.java.net/browse/JDK-8136763</a>
                                      looks incorrect.</div>
                                    <div>The corresponding manual test
                                      case fails on jdk8.</div>
                                    <div><br>
                                    </div>
                                    <div>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.</div>
                                    <div><br>
                                    </div>
                                    <div>If you for example copy:</div>
                                    <div>/Users/robin/Desktop/file1.txt</div>
                                    <div>/Users/robin/Desktop/file2.txt<br>
                                    </div>
                                    <div>/Users/robin/Desktop/file3.txt<br>
                                    </div>
                                    <div><br>
                                    </div>
                                    <div>the data in the transferable is</div>
                                    <div><br>
                                    </div>
                                    <div>/Users/robin/Desktop/file1.txt/Users/robin/Desktop/file2.txt<br>
                                    </div>
                                    <div>/Users/robin/Desktop/file3.txt<br>
                                    </div>
                                    <div><br>
                                    </div>
                                    <div>while the expected data is</div>
                                    <div><br>
                                    </div>
                                    <div>
                                      <div>/Users/robin/Desktop/file1.txt</div>
                                      <div>/Users/robin/Desktop/file2.txt<br>
                                      </div>
                                      <div>/Users/robin/Desktop/file3.txt<br>
                                      </div>
                                    </div>
                                    <div><br>
                                    </div>
                                    <div><br>
                                    </div>
                                    <div>This is also what you can
                                      observe when running the manual
                                      testcase.</div>
                                    <div><br>
                                    </div>
                                    <div>I have logged this in the bug
                                      database as issue JI-9041413.</div>
                                    <div><br>
                                    </div>
                                    <div>Attached you find a webrev with
                                      the proposed patch.</div>
                                    <div>The manual testcase succeeds
                                      with this patch, and fails
                                      without.</div>
                                    <div>At the bottom of this email,
                                      you also find the output of hg
                                      diff.</div>
                                    <div><br>
                                    </div>
                                    <div>The patch does not include a
                                      testcase, as there is already one
                                      available.</div>
                                    <div><br>
                                    </div>
                                    <div>Regards,</div>
                                    <div><br>
                                    </div>
                                    <div>Robin</div>
                                    <div><br>
                                    </div>
                                    <div><br>
                                    </div>
                                    <div>hg diff output:</div>
                                    <div><br>
                                    </div>
                                    <div>
                                      <div>diff -r 0844fa517c35
                                        src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java</div>
                                      <div>---
                                        a/src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java<span style="white-space:pre-wrap">   </span>Tue

                                        Jul 05 11:03:13 2016 -0700</div>
                                      <div>+++
                                        b/src/macosx/classes/sun/lwawt/macosx/CDataTransferer.java<span style="white-space:pre-wrap">   </span>Thu

                                        Jul 07 08:48:37 2016 +0200</div>
                                      <div>@@ -148,6 +148,7 @@</div>
                                      <div>             StringBuilder sb
                                        = new StringBuilder();</div>
                                      <div>           
                                         if(strings.length > 0) {</div>
                                      <div>               
                                         sb.append(strings[0]);</div>
                                      <div>+              
                                         sb.append(separator);</div>
                                      <div>                 for(int i =
                                        1; i < strings.length; i++) {</div>
                                      <div>                   
                                         sb.append(strings[i]);</div>
                                      <div>                   
                                         sb.append(separator);</div>
                                    </div>
                                  </div>
                                </blockquote>
                                <br>
                              </div>
                            </blockquote>
                          </div>
                          <br>
                        </div>
                      </blockquote>
                      <br>
                    </div>
                  </div>
                </div>
              </blockquote>
            </div>
            <br>
          </div>
        </div>
      </div>
    </blockquote>
    <br>
  </div></div></div>

</blockquote></div><br></div>