Fix: Hyperlinks break when removing rows

[shanto]

When removing a row from above cells with hyperlinks, HyperlinkCollection gets broken due to the reversed order of operation. The reversing of array (in current code) works as expected only when inserting rows ($pNumRows/Cols is positive), but breaks when removing rows ($pNumRows/Cols is negative). Therefore, reversing should be conditional. Not sure if this is the best solution, but it works. The same should most likely apply for other reference alteration codes in this file where array_reverse (or reverse order of operation) is used.

[MarkBaker]

Unable to replicate the problem that this fixes at present

[shanto]

Let's go thorough this test:

1. Insert hyperlinks to http://www.phpexcel.net in range A2:A5.
2. Then, we delete row 1 which brings A2:A5 up by one row, i.e. A1:A4.
3. At this point, you would expect all cells in range A1:A4 to contain the hyperlinks inserted earlier. Unfortunately, due to this bug, you are left with only one hyperlink at A1. Hyperlinks which are supposed to be present at A2:A4 are gone.

Tests/161hyperlinks-after-add-del-rows.php (new file)

<?php

/** Error reporting */
error_reporting(E_ALL);
ini_set('display_errors', TRUE);
ini_set('display_startup_errors', TRUE);
date_default_timezone_set('Europe/London');

define('EOL',(PHP_SAPI == 'cli') ? PHP_EOL : '<br />');

date_default_timezone_set('Europe/London');

/** Include path **/
set_include_path(get_include_path() . PATH_SEPARATOR . '../Classes/');

/** PHPExcel_IOFactory */
include 'PHPExcel/IOFactory.php';

$test = new PHPExcel();

$tab1 = $test->getSheet(0);
for($i = 2; $i <=5; $i++) {
    echo date('H:i:s') , " Setting Hyperlink to www.phpexcel.net at cell A{$i}" , EOL; 
    $cell = $tab1->getCell("A{$i}");
    $cell->setValue('www.phpexcel.net');
    $cell->getHyperlink()->setUrl('http://www.phpexcel.net')->setTooltip('link to http://www.phpexcel.net');
}

echo date('H:i:s') , " HyperlinkCollection before removing row 1 " , print_r($tab1->getHyperlinkCollection(), 1);

echo date('H:i:s') , " Removing row 1" , EOL;
$tab1->removeRow(1);

echo date('H:i:s') , " HyperlinkCollection after removing row 1 " , print_r($tab1->getHyperlinkCollection(), 1);

$out_file = str_replace('.php', '.xls', __FILE__);
echo date('H:i:s') , " Writing output" , EOL;
$objWriter = PHPExcel_IOFactory::createWriter($test, 'Excel5')->save($out_file);
echo date('H:i:s') , " File written to " , basename($out_file), EOL;

//system("open $out_file");

Sample output

05:03:06 Setting Hyperlink to www.phpexcel.net at cell A2
05:03:06 Setting Hyperlink to www.phpexcel.net at cell A3
05:03:06 Setting Hyperlink to www.phpexcel.net at cell A4
05:03:06 Setting Hyperlink to www.phpexcel.net at cell A5
05:03:06 HyperlinkCollection before removing row 1 Array
(
    [A2] => PHPExcel_Cell_Hyperlink Object
        (
            [_url:PHPExcel_Cell_Hyperlink:private] => http://www.phpexcel.net
            [_tooltip:PHPExcel_Cell_Hyperlink:private] => link to http://www.phpexcel.net
        )

    [A3] => PHPExcel_Cell_Hyperlink Object
        (
            [_url:PHPExcel_Cell_Hyperlink:private] => http://www.phpexcel.net
            [_tooltip:PHPExcel_Cell_Hyperlink:private] => link to http://www.phpexcel.net
        )

    [A4] => PHPExcel_Cell_Hyperlink Object
        (
            [_url:PHPExcel_Cell_Hyperlink:private] => http://www.phpexcel.net
            [_tooltip:PHPExcel_Cell_Hyperlink:private] => link to http://www.phpexcel.net
        )

    [A5] => PHPExcel_Cell_Hyperlink Object
        (
            [_url:PHPExcel_Cell_Hyperlink:private] => http://www.phpexcel.net
            [_tooltip:PHPExcel_Cell_Hyperlink:private] => link to http://www.phpexcel.net
        )

)
05:03:06 Removing row 1
05:03:06 HyperlinkCollection after removing row 1 Array
(
    [A1] => PHPExcel_Cell_Hyperlink Object
        (
            [_url:PHPExcel_Cell_Hyperlink:private] => http://www.phpexcel.net
            [_tooltip:PHPExcel_Cell_Hyperlink:private] => link to http://www.phpexcel.net
        )

)
05:03:06 Writing output
05:03:06 File written to 161hyperlinks-after-add-del-rows.xls

[MarkBaker]

OK thanks, I've recreated it now and merged manually for the moment. I'll run some further tests (e.g. removing row #3 rather than row #1, playing with columns, and with both columns and rows, and test the other collections as well. Thanks for highlighting the problem, and providing the sample code to demonstrate it as well.

[MarkBaker]

Slightly more complex. If I create the links in reverse order

for($i = 5; $i >=2; $i--) {

Then it breaks again!

I need to ensure that the collection is correctly sorted in the first place; and then reverse if necessary

[shanto]

How about this one?

// Update worksheet: hyperlinks
$aHyperlinkCollection = $pSheet->getHyperlinkCollection();
$aHyperlinkCollection_ = array();
foreach ($aHyperlinkCollection as $key => $value) {
    $newReference = $this->updateCellReference($key, $pBefore, $pNumCols, $pNumRows);
    $pSheet->setHyperLink($key, null);
    $aHyperlinkCollection_[$newReference] = $value;
}
foreach ($aHyperlinkCollection_ as $key => $value) {
    $pSheet->setHyperLink($key, $value);
}
unset($aHyperlinkCollection, $aHyperlinkCollection_);

I guess, this should be done for all code blocks in ReferenceHelper.php which touch *collections.

[MarkBaker]

I was looking more at:

$aHyperlinkCollection = $pSheet->getHyperlinkCollection();
($pNumCols > 0 || $pNumRows > 0) ? 
    uksort($aHyperlinkCollection, array('PHPExcel_ReferenceHelper', 'cellReverseSort')) : 
    uksort($aHyperlinkCollection, array('PHPExcel_ReferenceHelper', 'cellSort')); 
foreach ($aHyperlinkCollection as $key => $value) {
    $newReference = $this->updateCellReference($key, $pBefore, $pNumCols, $pNumRows);
    if ($key != $newReference) {
        $pSheet->setHyperlink( $newReference, $value );
        $pSheet->setHyperlink( $key, null );
    }
}

Where cellSort() and cellReverseSort() handle correct sorting of cell IDs by column and row; working on your principle of whether to array_reverse or not.

Initial tests look OK, but still checking the results.

[shanto]

Thanks for looking into this as well as other reference/collection alteration codes nearby. What you suggested above and later committed looks good, and seems to satisfy all 4 cases (Add|Remove x Row|Column) as in https://gist.github.com/Shanto/5430353.