Xls Writer Treat Hyperlink Starting with # as Internal

Fix #56, which had gone stale but is now reopened. The problem which it reported with Xlsx Writer was fixed long ago. However, Xls Writer has 2 problems regarding hyperlinks. First, some logic which should have been `if ... elseif ... else ...` was coded as `if ... if ... unconditional ...`, resulting in multiple writes and a corrupt worksheet (which Excel does fix correctly). Second, it treated a hyperlink url starting with `#` (pointer to a cell in the same spreadsheet) as external, when it should be treated as internal.

Also changed `Hyperlink::isInternal` to recognize a starting `#`, and to use `str_starts_with('sheet://')` rather than `str_contains`.

Author: oleibman

src/PhpSpreadsheet/Cell/Hyperlink.php

@@ -68,11 +68,11 @@ public function setTooltip(string $tooltip): static
     }
 
     /**
-     * Is this hyperlink internal? (to another worksheet).
+     * Is this hyperlink internal? (to another worksheet or a cell in this worksheet).
      */
     public function isInternal(): bool
     {
-        return str_contains($this->url, 'sheet://');
+        return str_starts_with($this->url, 'sheet://') || str_starts_with($this->url, '#');
     }
 
     public function getTypeHyperlink(): string

src/PhpSpreadsheet/Writer/Xls/Worksheet.php

@@ -458,8 +458,9 @@ public function close(): void
             [$column, $row] = Coordinate::indexesFromString($coordinate);
 
             $url = $hyperlink->getUrl();
-
-            if (str_contains($url, 'sheet://')) {
+            if ($url[0] === '#') {
+                $url = "internal:$url";
+            } elseif (str_starts_with($url, 'sheet://')) {
                 // internal to current workbook
                 $url = str_replace('sheet://', 'internal:', $url);
             } elseif (Preg::isMatch('/^(http:|https:|ftp:|mailto:)/', $url)) {
@@ -946,12 +947,11 @@ private function writeUrlRange(int $row1, int $col1, int $row2, int $col2, strin
         // Check for internal/external sheet links or default to web link
         if (Preg::isMatch('[^internal:]', $url)) {
             $this->writeUrlInternal($row1, $col1, $row2, $col2, $url);
-        }
-        if (Preg::isMatch('[^external:]', $url)) {
+        } elseif (Preg::isMatch('[^external:]', $url)) {
             $this->writeUrlExternal($row1, $col1, $row2, $col2, $url);
+        } else {
+            $this->writeUrlWeb($row1, $col1, $row2, $col2, $url);
         }
-
-        $this->writeUrlWeb($row1, $col1, $row2, $col2, $url);
     }
 
     /**

tests/PhpSpreadsheetTests/Writer/Xls/HyperlinkTest.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xls;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class HyperlinkTest extends AbstractFunctional
+{
+    public function testHyperlink(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet1 = $spreadsheet->getActiveSheet();
+        $sheet1->setTitle('First');
+        $sheet2 = $spreadsheet->createSheet();
+        $sheet2->setTitle('Second');
+        $sheet2->setCellValue('A100', 'other sheet');
+        $sheet1->setCellValue('A100', 'this sheet');
+        $sheet1->setCellValue('A1', '=HYPERLINK("#A100", "here")');
+        $sheet1->setCellValue('A2', '=HYPERLINK("#Second!A100", "there")');
+        $sheet1->setCellValue('A3', '=HYPERLINK("http://example.com", "external")');
+        $sheet1->setCellValue('A4', 'gotoA101');
+        $sheet1->getCell('A4')
+            ->getHyperlink()
+            ->setUrl('#A101');
+
+        $robj = $this->writeAndReload($spreadsheet, 'Xls');
+        $spreadsheet->disconnectWorksheets();
+        $sheet0 = $robj->setActiveSheetIndex(0);
+        self::assertSame('sheet://#A100', $sheet0->getCell('A1')->getHyperlink()->getUrl());
+        self::assertSame('sheet://#Second!A100', $sheet0->getCell('A2')->getHyperlink()->getUrl());
+        self::assertSame('http://example.com', $sheet0->getCell('A3')->getHyperlink()->getUrl());
+        self::assertSame('sheet://#A101', $sheet0->getCell('A4')->getHyperlink()->getUrl());
+        $robj->disconnectWorksheets();
+    }
+}

tests/PhpSpreadsheetTests/Writer/Xlsx/HyperlinkTest.php

@@ -0,0 +1,38 @@
+<?php
+
+declare(strict_types=1);
+
+namespace PhpOffice\PhpSpreadsheetTests\Writer\Xlsx;
+
+use PhpOffice\PhpSpreadsheet\Spreadsheet;
+use PhpOffice\PhpSpreadsheetTests\Functional\AbstractFunctional;
+
+class HyperlinkTest extends AbstractFunctional
+{
+    public function testHyperlink(): void
+    {
+        $spreadsheet = new Spreadsheet();
+        $sheet1 = $spreadsheet->getActiveSheet();
+        $sheet1->setTitle('First');
+        $sheet2 = $spreadsheet->createSheet();
+        $sheet2->setTitle('Second');
+        $sheet2->setCellValue('A100', 'other sheet');
+        $sheet1->setCellValue('A100', 'this sheet');
+        $sheet1->setCellValue('A1', '=HYPERLINK("#A100", "here")');
+        $sheet1->setCellValue('A2', '=HYPERLINK("#Second!A100", "there")');
+        $sheet1->setCellValue('A3', '=HYPERLINK("http://example.com", "external")');
+        $sheet1->setCellValue('A4', 'gotoA101');
+        $sheet1->getCell('A4')
+            ->getHyperlink()
+            ->setUrl('#A101');
+
+        $robj = $this->writeAndReload($spreadsheet, 'Xlsx');
+        $spreadsheet->disconnectWorksheets();
+        $sheet0 = $robj->setActiveSheetIndex(0);
+        self::assertSame('sheet://#A100', $sheet0->getCell('A1')->getHyperlink()->getUrl());
+        self::assertSame('sheet://#Second!A100', $sheet0->getCell('A2')->getHyperlink()->getUrl());
+        self::assertSame('http://example.com', $sheet0->getCell('A3')->getHyperlink()->getUrl());
+        self::assertSame('sheet://#A101', $sheet0->getCell('A4')->getHyperlink()->getUrl());
+        $robj->disconnectWorksheets();
+    }
+}