FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

fix: respect explicit spreadsheet preview type by SpencerZhang · Pull Request #772 · kekingcn/kkFileView · GitHub

/ kkFileView Public

fix: respect explicit spreadsheet preview type - #772

Open
SpencerZhang wants to merge 1 commit into
kekingcn:masterfrom
SpencerZhang:fix/xlsx-pdf-preview-routing
ConversationCommits (1)ChecksFiles changed
Open

fix: respect explicit spreadsheet preview type#772
SpencerZhang wants to merge 1 commit into
kekingcn:masterfrom
SpencerZhang:fix/xlsx-pdf-preview-routing

Conversation

SpencerZhang commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

What changed

  • Track whether officePreviewType was explicitly supplied by the request or caller.
  • Respect explicit pdf, html, xlsx, and csv preview routing for spreadsheet files.
  • Preserve the existing default Web-preview behavior when no preview type is explicitly supplied.
  • Add focused unit tests for cache-target selection and Web-preview routing.

Why

When office.type.web=web, an explicit officePreviewType=pdf request for an xlsx file could still be routed to the xlsx Web preview. Spreadsheet cache naming was also decided before the request-level preview type was applied, so the generated artifact could disagree with the requested mode.

This change distinguishes an explicit request override from the configured default and applies it before selecting the cache target and preview page.

Impact

  • Explicit xlsx PDF requests now use the Office-to-PDF flow.
  • Explicit HTML requests continue to generate HTML previews.
  • Requests without an explicit preview type retain the existing xlsx/csv Web-preview behavior.

Validation

  • git diff --check
  • Added FileHandlerServiceTests and OfficeFilePreviewImplTests.
  • Targeted Maven tests could not run because com.aspose:aspose-cad:25.10 was unavailable from the configured Hzero Maven repository.

klboke requested changes Jul 16, 2026
View reviewed changes

klboke left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Requesting changes for one routing blocker.

OfficeFilePreviewImpl.filePreviewHandle() calls checkAndHandleConvertStatus() before shouldUseWebPreview(). With this patch, an explicit officePreviewType=xlsx or csv uses isHtmlView=false, so it receives the same .pdf cache/status key used by PDF/image conversion. If that spreadsheet already has an image conversion in progress, failed, or timed out, the explicit Web-preview request is intercepted by the conversion-status check and returns the waiting/error page instead of the xlsx/csv Web preview.

I reproduced this against the full filePreviewHandle() path: after marking the shared .pdf cache key as converting, an explicit xlsx request returned waiting rather than officeweb.

Please handle shouldUseWebPreview(fileAttribute) before checking server-side conversion status, since the Web preview does not depend on that conversion. Please also add a regression test that exercises the full filePreviewHandle() ordering; the current helper-only tests do not catch this case.

The four GitHub checks are green, and the eight newly added focused tests pass locally.

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

klboke klboke requested changes

Requested changes must be addressed to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

Back | FazBrowse Home | New Git URL