mirror of
https://github.com/PHPOffice/PhpSpreadsheet.git
synced 2026-09-11 02:26:28 +00:00
Fix Some Hyperlink Problems
Fix #3889. Fix #2464. After evaluating HYPERLINK function specifying cell address(es) rather than literal(s), the calculated hyperlink is attached to the wrong cell. I probably should have figured this out sooner - this is just another instance of the common problem of trying to assign a cell to a variable and then using the variable after a delay during which the "current" cell may have changed. The problem is resolved by storing the cell's worksheet and coordinate on entry to HYPERLINK, then restoring those before manipulating the hyperlink before returning to the caller. An additional problem with hyperlinks is that assigning a new value to a cell has not cleared any hyperlink associated with the cell. This will now happen, if needed, whenever `setValue` or `setValueExplicit` is called. A third problem is unaddressed. A (fairly unrealistic) formula like: ```php $sheet->getCell('A4')->setValue('=LEN(HYPERLINK("http://www.example.com", "Example"))'); ``` winds up attaching a hyperlink to the cell when its value is calculated, and it probably shouldn't. However, @TobiasBg reports in 2464 that Excel for Mac does the same thing, and I've just confirmed that Excel 365 on Windows does likewise. Furthermore, the hyperlink which Excel creates is not usable ("Cannot open the specified file"), whereas the link that PhpSpreadsheet creates is usable. So we're not doing any worse than Excel, and arguably doing better. I'm satisfied.
This commit is contained in:
@@ -23,6 +23,13 @@ class Hyperlink
|
||||
*/
|
||||
public static function set(mixed $linkURL = '', mixed $displayName = null, ?Cell $cell = null): string
|
||||
{
|
||||
$worksheet = null;
|
||||
$coordinate = '';
|
||||
if ($cell !== null) {
|
||||
$coordinate = $cell->getCoordinate();
|
||||
$worksheet = $cell->getWorksheetOrNull();
|
||||
}
|
||||
|
||||
$linkURL = ($linkURL === null) ? '' : StringHelper::convertToString(Functions::flattenSingleValue($linkURL));
|
||||
$displayName = ($displayName === null) ? '' : Functions::flattenSingleValue($displayName);
|
||||
|
||||
@@ -38,9 +45,11 @@ class Hyperlink
|
||||
$displayName = $linkURL;
|
||||
}
|
||||
|
||||
$cell->getHyperlink()
|
||||
->setUrl($linkURL);
|
||||
$cell->getHyperlink()->setTooltip($displayName);
|
||||
$worksheet?->getCell($coordinate)
|
||||
->getHyperlink()
|
||||
->setUrl($linkURL)
|
||||
->setTooltip($displayName)
|
||||
->setDisplay('');
|
||||
|
||||
return $displayName;
|
||||
}
|
||||
|
||||
@@ -233,6 +233,9 @@ class Cell implements Stringable
|
||||
*/
|
||||
public function setValue(mixed $value, ?IValueBinder $binder = null): self
|
||||
{
|
||||
if ($this->hadHyperlink) {
|
||||
$this->clearHyperlink();
|
||||
}
|
||||
// Cells?->Worksheet?->Spreadsheet
|
||||
$binder ??= $this->parent?->getParent()?->getParent()?->getValueBinder() ?? self::getValueBinder();
|
||||
if (!$binder->bindValue($this, $value)) {
|
||||
@@ -242,6 +245,24 @@ class Cell implements Stringable
|
||||
return $this;
|
||||
}
|
||||
|
||||
private bool $hadHyperlink = false;
|
||||
|
||||
/** @internal */
|
||||
public function setHadHyperlink(bool $hadHyperlink): void
|
||||
{
|
||||
$this->hadHyperlink = $hadHyperlink;
|
||||
}
|
||||
|
||||
private function clearHyperlink(): void
|
||||
{
|
||||
$worksheet = $this->getWorksheetOrNull();
|
||||
if ($worksheet !== null) {
|
||||
$coordinate = $this->getCoordinate();
|
||||
$worksheet->setHyperlink($coordinate, null);
|
||||
}
|
||||
$this->hadHyperlink = false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Set the value for a cell, with the explicit data type passed to the method (bypassing any use of the value binder).
|
||||
*
|
||||
@@ -259,6 +280,9 @@ class Cell implements Stringable
|
||||
*/
|
||||
public function setValueExplicit(mixed $value, string $dataType = DataType::TYPE_STRING): self
|
||||
{
|
||||
if ($this->hadHyperlink) {
|
||||
$this->clearHyperlink();
|
||||
}
|
||||
$oldValue = $this->value;
|
||||
$quotePrefix = false;
|
||||
|
||||
@@ -748,7 +772,8 @@ class Cell implements Stringable
|
||||
throw new SpreadsheetException('Cannot get hyperlink for cell that is not bound to a worksheet');
|
||||
}
|
||||
|
||||
return $this->getWorksheet()->getHyperlink($this->getCoordinate());
|
||||
return $this->getWorksheet()
|
||||
->getHyperlink($this->getCoordinate());
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -762,7 +787,8 @@ class Cell implements Stringable
|
||||
throw new SpreadsheetException('Cannot set hyperlink for cell that is not bound to a worksheet');
|
||||
}
|
||||
|
||||
$this->getWorksheet()->setHyperlink($this->getCoordinate(), $hyperlink);
|
||||
$this->getWorksheet()
|
||||
->setHyperlink($this->getCoordinate(), $hyperlink);
|
||||
|
||||
return $this->updateInCollection();
|
||||
}
|
||||
|
||||
@@ -321,13 +321,24 @@ class Html extends BaseReader
|
||||
}
|
||||
|
||||
//catching the Exception and ignoring the invalid data types
|
||||
$hyperlink = null;
|
||||
if ($sheet->hyperlinkExists($column . $row)) {
|
||||
$hyperlink = $sheet->getHyperlink($column . $row);
|
||||
}
|
||||
|
||||
try {
|
||||
$sheet->setCellValueExplicit($column . $row, $cellContent, $attributeArray['data-type']);
|
||||
} catch (SpreadsheetException) {
|
||||
$sheet->setCellValue($column . $row, $cellContent);
|
||||
}
|
||||
$sheet->setHyperlink($column . $row, $hyperlink);
|
||||
} else {
|
||||
$hyperlink = null;
|
||||
if ($sheet->hyperlinkExists($column . $row)) {
|
||||
$hyperlink = $sheet->getHyperlink($column . $row);
|
||||
}
|
||||
$sheet->setCellValue($column . $row, $cellContent);
|
||||
$sheet->setHyperlink($column . $row, $hyperlink);
|
||||
}
|
||||
$this->dataArray[$row][$column] = $cellContent; // @phpstan-ignore-line
|
||||
}
|
||||
|
||||
@@ -510,7 +510,18 @@ class Xml extends BaseReader
|
||||
$cellDataFormula = AddressHelper::convertFormulaToA1($cellDataFormula, $rowID, $columnNumber);
|
||||
}
|
||||
|
||||
$spreadsheet->getActiveSheet()->getCell($columnID . $rowID)->setValueExplicit((($hasCalculatedValue) ? $cellDataFormula : $cellValue), $type);
|
||||
$hyperlink = null;
|
||||
if ($spreadsheet->getActiveSheet()->hyperlinkExists($columnID . $rowID)) {
|
||||
$hyperlink = $spreadsheet->getActiveSheet()->getHyperlink($columnID . $rowID);
|
||||
}
|
||||
$spreadsheet->getActiveSheet()
|
||||
->getCell($columnID . $rowID)
|
||||
->setValueExplicit(
|
||||
$hasCalculatedValue ? $cellDataFormula : $cellValue,
|
||||
$type
|
||||
);
|
||||
$spreadsheet->getActiveSheet()
|
||||
->setHyperlink($columnID . $rowID, $hyperlink);
|
||||
if ($hasCalculatedValue) {
|
||||
$spreadsheet->getActiveSheet()->getCell($columnID . $rowID)->setCalculatedValue($cellValue, $originalType === DataType::TYPE_NUMERIC);
|
||||
}
|
||||
|
||||
@@ -1168,7 +1168,7 @@ class ReferenceHelper
|
||||
private function clearStripCell(Worksheet $worksheet, string $coordinate): void
|
||||
{
|
||||
$worksheet->removeConditionalStyles($coordinate);
|
||||
$worksheet->setHyperlink($coordinate);
|
||||
$worksheet->setHyperlink($coordinate, null, false);
|
||||
$worksheet->setDataValidation($coordinate);
|
||||
$worksheet->removeComment($coordinate);
|
||||
|
||||
|
||||
@@ -3449,6 +3449,7 @@ class Worksheet
|
||||
*/
|
||||
public function getHyperlink(string $cellCoordinate): Hyperlink
|
||||
{
|
||||
$this->getCell($cellCoordinate)->setHadHyperlink(true);
|
||||
// return hyperlink if we already have one
|
||||
if (isset($this->hyperlinkCollection[$cellCoordinate])) {
|
||||
return $this->hyperlinkCollection[$cellCoordinate];
|
||||
@@ -3467,12 +3468,17 @@ class Worksheet
|
||||
*
|
||||
* @return $this
|
||||
*/
|
||||
public function setHyperlink(string $cellCoordinate, ?Hyperlink $hyperlink = null): static
|
||||
public function setHyperlink(string $cellCoordinate, ?Hyperlink $hyperlink = null, bool $reset = true): static
|
||||
{
|
||||
if ($hyperlink === null) {
|
||||
unset($this->hyperlinkCollection[$cellCoordinate]);
|
||||
if ($reset) {
|
||||
$this->getCell($cellCoordinate)
|
||||
->setHadHyperlink(false);
|
||||
}
|
||||
} else {
|
||||
$this->hyperlinkCollection[$cellCoordinate] = $hyperlink;
|
||||
$this->getCell($cellCoordinate)->setHadHyperlink(true);
|
||||
}
|
||||
|
||||
return $this;
|
||||
|
||||
@@ -0,0 +1,156 @@
|
||||
<?php
|
||||
|
||||
declare(strict_types=1);
|
||||
|
||||
namespace PhpOffice\PhpSpreadsheetTests\Cell;
|
||||
|
||||
use PhpOffice\PhpSpreadsheet\Cell\DataType;
|
||||
use PhpOffice\PhpSpreadsheet\Spreadsheet;
|
||||
use PHPUnit\Framework\TestCase;
|
||||
|
||||
class Hyperlink2Test extends TestCase
|
||||
{
|
||||
public static function testTwoLiterals(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.net';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', "=HYPERLINK(\"$actualUrl\", \"$toolTip\")");
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
// No hyperlink should be created for A1 or B1 - issue 3889
|
||||
$hyperlink = $sheet->getCell('A1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$hyperlink = $sheet->getCell('B1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public static function testCellAndLiteral(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.org';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', "=HYPERLINK(A1, \"$toolTip\")");
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
// No hyperlink should be created for A1 or B1 - issue 3889
|
||||
$hyperlink = $sheet->getCell('A1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$hyperlink = $sheet->getCell('B1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public static function testLiteralAndCell(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.edu';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', "=HYPERLINK(\"$actualUrl\", B1)");
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
// No hyperlink should be created for A1 or B1 - issue 3889
|
||||
$hyperlink = $sheet->getCell('A1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$hyperlink = $sheet->getCell('B1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public static function testTwoCells(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.com';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', '=HYPERLINK(A1, B1)');
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
// No hyperlink should be created for A1 or B1 - issue 3889
|
||||
$hyperlink = $sheet->getCell('A1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$hyperlink = $sheet->getCell('B1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
$spreadsheet->disconnectWorksheets();
|
||||
}
|
||||
|
||||
public static function testResetOnSet(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.com';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', '=HYPERLINK(A1, B1)');
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
|
||||
$sheet->setCellValue('C1', 123);
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
}
|
||||
|
||||
public static function testResetOnSetExplicit(): void
|
||||
{
|
||||
$spreadsheet = new Spreadsheet();
|
||||
$sheet = $spreadsheet->getActiveSheet();
|
||||
$basicUrl = 'example.com';
|
||||
$actualUrl = "https://www.$basicUrl";
|
||||
$toolTip = "Link to $basicUrl";
|
||||
$sheet->setCellValue('A1', $actualUrl);
|
||||
$sheet->setCellValue('B1', $toolTip);
|
||||
$sheet->setCellValue('C1', '=HYPERLINK(A1, B1)');
|
||||
$result = $sheet->getCell('C1')->getCalculatedValue();
|
||||
self::assertSame($toolTip, $sheet->getCell('C1')->getCalculatedValue());
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame($actualUrl, $hyperlink->getUrl());
|
||||
self::assertSame($toolTip, $hyperlink->getTooltip());
|
||||
|
||||
$sheet->setCellValueExplicit('C1', '123', DataType::TYPE_STRING);
|
||||
$hyperlink = $sheet->getCell('C1')->getHyperlink();
|
||||
self::assertSame('', $hyperlink->getUrl());
|
||||
self::assertSame('', $hyperlink->getTooltip());
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user