Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Set font size to 10 when given 0 #2100

Merged
merged 1 commit into from
May 29, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ and this project adheres to [Semantic Versioning](https://semver.org).
- Fixed issue with Xlsx@listWorksheetInfo not returning any data
- Fixed invalid arguments triggering mb_substr() error in LEFT(), MID() and RIGHT() text functions. [Issue #640](https://github.com/PHPOffice/PhpSpreadsheet/issues/640)
- Fix for [Issue #1916](https://github.com/PHPOffice/PhpSpreadsheet/issues/1916) - Invalid signature check for XML files
- Fix change in `Font::setSize()` behavior for PHP8. [PR #2100](https://github.com/PHPOffice/PhpSpreadsheet/pull/2100)

## 1.17.1 - 2021-03-01

Expand Down
13 changes: 10 additions & 3 deletions src/PhpSpreadsheet/Style/Font.php
Original file line number Diff line number Diff line change
Expand Up @@ -249,15 +249,22 @@ public function getSize()
/**
* Set Size.
*
* @param float $pValue
* @param mixed $pValue A float representing the value of a positive measurement in points (1/72 of an inch)
*
* @return $this
*/
public function setSize($pValue)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docblock needs modifying to mixed to keep phpstan happy, and it looks like phpcs is complaining about trailing white space in the comment lines.
If you can just make those changes, then the PR is good to merge

{
if ($pValue == '') {
$pValue = 10;
if (is_string($pValue) || is_int($pValue)) {
$pValue = (float) $pValue; // $pValue = 0 if given string is not numeric
}

// Size must be a positive floating point number
// ECMA-376-1:2016, part 1, chapter 18.4.11 sz (Font Size), p. 1536
if (!is_float($pValue) || !($pValue > 0)) {
$pValue = 10.0;
}

if ($this->isSupervisor) {
$styleArray = $this->getStyleArray(['size' => $pValue]);
$this->getActiveSheet()->getStyle($this->getSelectedCells())->applyFromArray($styleArray);
Expand Down
32 changes: 32 additions & 0 deletions tests/PhpSpreadsheetTests/Style/FontTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -39,4 +39,36 @@ public function testSuperSubScript(): void
self::assertTrue($font->getSuperscript());
self::assertFalse($font->getSubscript(), 'False remains unchanged');
}

public function testSize(): void
{
$spreadsheet = new Spreadsheet();
$sheet = $spreadsheet->getActiveSheet();
$cell = $sheet->getCell('A1');
$cell->setValue('Cell A1');
$font = $cell->getStyle()->getFont();

self::assertEquals(11, $font->getSize(), 'The default is 11');

$font->setSize(12);
self::assertEquals(12, $font->getSize(), 'Accepted new font size');

$invalidFontSizeValues = [
'',
false,
true,
'non_numeric_string',
'-1.0',
-1.0,
0,
[],
(object) [],
null,
];
foreach ($invalidFontSizeValues as $invalidFontSizeValue) {
$font->setSize(12);
$font->setSize($invalidFontSizeValue);
self::assertEquals(10, $font->getSize(), 'Set to 10 after trying to set an invalid value.');
}
}
MarkBaker marked this conversation as resolved.
Show resolved Hide resolved
}