fix: use png as preview right away

The initial office preview implementation converted an office document with LibreOffice to PDF, used ImageMagick to extract the first page as JPEG, and passed it OC_Image. nextcloud#10198 changed the implementation to use PNG rather than PDF. OC_Image can use a PNG as a preview right away, so the ImageMagick step is unnecessary. The registration code was updated to not ask ImageMagick if PDF is supported, as PDFs are no longer used to create office document previews. Signed-off-by: Daniel Kesselberg <[email protected]>
zak39 · Dec 19, 2023 · 3e8e8be · 3e8e8be
1 parent 8cb3d4b
commit 3e8e8be
Show file tree

Hide file tree

Showing 2 changed files with 43 additions and 37 deletions.
diff --git a/lib/private/Preview/Office.php b/lib/private/Preview/Office.php
@@ -33,7 +33,6 @@
 use OCP\IImage;
 use OCP\ITempManager;
 use OCP\Server;
-use Psr\Log\LoggerInterface;
 
 abstract class Office extends ProviderV2 {
 	/**
@@ -93,30 +92,18 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 			return null;
 		}
 
-		try {
-			$filename = $outdir . pathinfo($absPath, PATHINFO_FILENAME) . '.png';
-
-			$png = new \Imagick($filename . '[0]');
-			$png->setImageFormat('jpg');
-		} catch (\Exception $e) {
-			$this->cleanTmpFiles();
-			\OC::$server->get(LoggerInterface::class)->error($e->getMessage(), [
-				'exception' => $e,
-				'app' => 'core',
-			]);
-			return null;
-		}
+		$preview = $outdir . pathinfo($absPath, PATHINFO_FILENAME) . '.png';
 
 		$image = new \OCP\Image();
-		$image->loadFromData((string) $png);
+		$image->loadFromFile($preview);
 
 		$this->cleanTmpFiles();
 
 		if ($image->valid()) {
 			$image->scaleDownToFit($maxX, $maxY);
-
 			return $image;
 		}
+
 		return null;
 	}
 }
diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php
@@ -366,7 +366,7 @@ protected function registerCoreProviders() {
 		$this->registerCoreProvider(Preview\OpenDocument::class, '/application\/vnd.oasis.opendocument.*/');
 		$this->registerCoreProvider(Preview\Imaginary::class, Preview\Imaginary::supportedMimeTypes());
 
-		// SVG, Office and Bitmap require imagick
+		// SVG and Bitmap require imagick
 		if ($this->imagickSupport->hasExtension()) {
 			$imagickProviders = [
 				'SVG' => ['mimetype' => '/image\/svg\+xml/', 'class' => Preview\SVG::class],
@@ -391,28 +391,10 @@ protected function registerCoreProviders() {
 					$this->registerCoreProvider($class, $provider['mimetype']);
 				}
 			}
-
-			if ($this->imagickSupport->supportsFormat('PDF')) {
-				// Office requires openoffice or libreoffice
-				$officeBinary = $this->config->getSystemValue('preview_libreoffice_path', null);
-				if (!is_string($officeBinary)) {
-					$officeBinary = $this->binaryFinder->findBinaryPath('libreoffice');
-				}
-				if (!is_string($officeBinary)) {
-					$officeBinary = $this->binaryFinder->findBinaryPath('openoffice');
-				}
-
-				if (is_string($officeBinary)) {
-					$this->registerCoreProvider(Preview\MSOfficeDoc::class, '/application\/msword/', ["officeBinary" => $officeBinary]);
-					$this->registerCoreProvider(Preview\MSOffice2003::class, '/application\/vnd.ms-.*/', ["officeBinary" => $officeBinary]);
-					$this->registerCoreProvider(Preview\MSOffice2007::class, '/application\/vnd.openxmlformats-officedocument.*/', ["officeBinary" => $officeBinary]);
-					$this->registerCoreProvider(Preview\OpenDocument::class, '/application\/vnd.oasis.opendocument.*/', ["officeBinary" => $officeBinary]);
-					$this->registerCoreProvider(Preview\StarOffice::class, '/application\/vnd.sun.xml.*/', ["officeBinary" => $officeBinary]);
-					$this->registerCoreProvider(Preview\EMF::class, '/image\/emf/', ['officeBinary' => $officeBinary]);
-				}
-			}
 		}
 
+		$this->registerCoreProvidersOffice();
+
 		// Video requires avconv or ffmpeg
 		if (in_array(Preview\Movie::class, $this->getEnabledDefaultProvider())) {
 			$movieBinary = $this->config->getSystemValue('preview_ffmpeg_path', null);
@@ -430,6 +412,43 @@ protected function registerCoreProviders() {
 		}
 	}
 
+	private function registerCoreProvidersOffice(): void {
+		$officeProviders = [
+			['mimetype' => '/application\/msword/', 'class' => Preview\MSOfficeDoc::class],
+			['mimetype' => '/application\/vnd.ms-.*/', 'class' => Preview\MSOffice2003::class],
+			['mimetype' => '/application\/vnd.openxmlformats-officedocument.*/', 'class' => Preview\MSOffice2007::class],
+			['mimetype' => '/application\/vnd.oasis.opendocument.*/', 'class' => Preview\OpenDocument::class],
+			['mimetype' => '/application\/vnd.sun.xml.*/', 'class' => Preview\StarOffice::class],
+			['mimetype' => '/image\/emf/', 'class' => Preview\EMF::class],
+		];
+
+		$findBinary = true;
+		$officeBinary = false;
+
+		foreach ($officeProviders as $provider) {
+			$class = $provider['class'];
+			if (!in_array(trim($class, '\\'), $this->getEnabledDefaultProvider())) {
+				continue;
+			}
+
+			if ($findBinary) {
+				// Office requires openoffice or libreoffice
+				$officeBinary = $this->config->getSystemValue('preview_libreoffice_path', false);
+				if ($officeBinary === false) {
+					$officeBinary = $this->binaryFinder->findBinaryPath('libreoffice');
+				}
+				if ($officeBinary === false) {
+					$officeBinary = $this->binaryFinder->findBinaryPath('openoffice');
+				}
+				$findBinary = false;
+			}
+
+			if ($officeBinary) {
+				$this->registerCoreProvider($class, $provider['mimetype'], ['officeBinary' => $officeBinary]);
+			}
+		}
+	}
+
 	private function registerBootstrapProviders(): void {
 		$context = $this->bootstrapCoordinator->getRegistrationContext();