-
-
Notifications
You must be signed in to change notification settings - Fork 7
feat(PdfReader): add Document properties Function #751
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -74,7 +74,9 @@ | |
| } | ||
| @if (ShowTwoPagesOneView || ShowPresentationMode) | ||
| { | ||
| <Divider></Divider> | ||
| <div class="divider"> | ||
| <div class="divider-mask"></div> | ||
| </div> | ||
| } | ||
| <div class="dropdown-item dropdown-item-doc"> | ||
| <i class="@_dropdownItemDefaultIcon"></i> | ||
|
|
@@ -96,4 +98,80 @@ | |
| </div> | ||
| </div> | ||
| </div> | ||
| <div class="bb-view-pdf-info"> | ||
| <div class="bb-view-pdf-backdrop"></div> | ||
| <div class="bb-view-pdf-dialog"> | ||
| <div class="bb-view-pdf-dialog-title mb-3">Document properties</div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">File name:</div> | ||
| <div class="bb-view-pdf-dialog-filename"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">File size:</div> | ||
| <div class="bb-view-pdf-dialog-filesize"></div> | ||
| </div> | ||
| <div class="divider"> | ||
| <div class="divider-mask"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Title:</div> | ||
| <div class="bb-view-pdf-dialog-title"></div> | ||
|
Comment on lines
+104
to
+118
|
||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Author:</div> | ||
| <div class="bb-view-pdf-dialog-author"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Subject:</div> | ||
| <div class="bb-view-pdf-dialog-subject"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Keywords:</div> | ||
| <div class="bb-view-pdf-dialog-keywords"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Created:</div> | ||
| <div class="bb-view-pdf-dialog-created"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Modified:</div> | ||
| <div class="bb-view-pdf-dialog-modified"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Application:</div> | ||
| <div class="bb-view-pdf-dialog-application"></div> | ||
| </div> | ||
| <div class="divider"> | ||
| <div class="divider-mask"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">PDF producer:</div> | ||
| <div class="bb-view-pdf-dialog-producer"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">PDF version:</div> | ||
| <div class="bb-view-pdf-dialog-version"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Page count:</div> | ||
| <div class="bb-view-pdf-dialog-count"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Page size:</div> | ||
| <div class="bb-view-pdf-dialog-size"></div> | ||
| </div> | ||
| <div class="divider"> | ||
| <div class="divider-mask"></div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-item"> | ||
| <div class="bb-view-pdf-dialog-label">Fast web view:</div> | ||
| <div class="bb-view-pdf-dialog-webview">No</div> | ||
| </div> | ||
| <div class="bb-view-pdf-dialog-close"> | ||
| <button type="button" class="btn btn-primary btn-close-doc"> | ||
| <span>@Localizer["CloseButtonText"]</span> | ||
| </button> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -5,7 +5,7 @@ import Data from '../BootstrapBlazor/modules/data.js'; | |||||||||||||||||||||||||||
| import EventHandler from '../BootstrapBlazor/modules/event-handler.js'; | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| if (pdfjsLib != null) { | ||||||||||||||||||||||||||||
| pdfjsLib.GlobalWorkerOptions.workerSrc = './pdf.worker.min.mjs'; | ||||||||||||||||||||||||||||
| pdfjsLib.GlobalWorkerOptions.workerSrc = '/_content/BootstrapBlazor.PdfReader/lib/pdf.worker.min.mjs'; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| export async function init(id, invoke, options) { | ||||||||||||||||||||||||||||
|
|
@@ -101,9 +101,120 @@ const loadPdf = async (el, invoke, options) => { | |||||||||||||||||||||||||||
| const pdfDocument = await loadingTask.promise; | ||||||||||||||||||||||||||||
| pdfViewer.setDocument(pdfDocument); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| pdfDocument.getMetadata().then(metadata => { | ||||||||||||||||||||||||||||
| loadMetadata(el, pdfViewer, metadata); | ||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| return pdfViewer; | ||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||
| const loadMetadata = (el, pdfViewer, metadata) => { | ||||||||||||||||||||||||||||
| console.log(metadata); | ||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||
| console.log(metadata); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): Multiple queried metadata elements are never populated, so the dialog will remain empty for those fields.
In loadMetadata, title, author, subject, and keywords are queried but never assigned from metadata, so those dialog rows will always be blank even when the PDF provides values. Please populate them from metadata (e.g. title.textContent = metadata.info.Title || '') so all available metadata is shown.
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variables title, author, subject, and keywords are declared but never used. They should either be populated with metadata values (e.g., title.textContent = metadata.info.Title) or removed if not needed.
| const keywords = el.querySelector('.bb-view-pdf-dialog-keywords'); | |
| const keywords = el.querySelector('.bb-view-pdf-dialog-keywords'); | |
| if (title) title.textContent = metadata.info.Title || ''; | |
| if (author) author.textContent = metadata.info.Author || ''; | |
| if (subject) subject.textContent = metadata.info.Subject || ''; | |
| if (keywords) keywords.textContent = metadata.info.Keywords || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue: Direct access to metadata.info.* assumes all properties exist and can throw when metadata is missing.
In loadMetadata, please guard these accesses with metadata && metadata.info and use optional chaining or fallbacks when assigning to textContent, e.g. created.textContent = metadata.info?.CreationDate ? parsePdfDate(metadata.info.CreationDate)?.toLocaleString() : '' and similar for the other fields.
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code doesn't handle cases where metadata.info.CreationDate might be undefined or null. Consider adding a null check or providing a fallback value to avoid displaying "undefined" or causing an error: created.textContent = parsePdfDate(metadata.info.CreationDate)?.toLocaleString() || 'N/A'
| created.textContent = parsePdfDate(metadata.info.CreationDate)?.toLocaleString(); | |
| created.textContent = parsePdfDate(metadata.info.CreationDate)?.toLocaleString() || 'N/A'; |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable modified is declared but never used. It should be populated with metadata.info.ModDate (e.g., modified.textContent = parsePdfDate(metadata.info.ModDate)?.toLocaleString()) or removed if not needed.
| const modified = el.querySelector('.bb-view-pdf-dialog-modified'); | |
| const modified = el.querySelector('.bb-view-pdf-dialog-modified'); | |
| modified.textContent = parsePdfDate(metadata.info.ModDate)?.toLocaleString(); |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference error if metadata.info.Creator is undefined. Add a null check or use optional chaining: application.textContent = metadata.info.Creator || ''
| application.textContent = metadata.info.Creator; | |
| application.textContent = metadata.info?.Creator || ''; |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference error if metadata.info.Producer is undefined. Add a null check or use optional chaining: producer.textContent = metadata.info.Producer || ''
| producer.textContent = metadata.info.Producer; | |
| producer.textContent = metadata.info?.Producer || ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
suggestion (bug_risk): The modified and webview elements are queried but never used, leaving the dialog fields inconsistent.
.bb-view-pdf-dialog-modified and .bb-view-pdf-dialog-webview are queried, but modified.textContent is never set and webview is never used (Razor currently hardcodes No). This leaves “Modified” always blank and makes the webview query dead code. Either populate modified (e.g. from metadata.info.ModDate via parsePdfDate) and drive webview from real document data, or remove these queries to avoid inconsistent UI and unused elements.
Suggested implementation:
const created = el.querySelector('.bb-view-pdf-dialog-created');
created.textContent = parsePdfDate(metadata.info.CreationDate)?.toLocaleString();
const modified = el.querySelector('.bb-view-pdf-dialog-modified');
modified.textContent = parsePdfDate(metadata.info.ModDate)?.toLocaleString();
const application = el.querySelector('.bb-view-pdf-dialog-application');- Locate where
.bb-view-pdf-dialog-webviewis queried inPdfReader.razor.jsand either:- Bind it to a real data source (e.g. a boolean or flag in
metadatathat indicates whether the document was viewed via web), or - Remove the query and any associated unused variable if no such data is available yet.
- Bind it to a real data source (e.g. a boolean or flag in
- Ensure the Razor markup for the “Modified” field is present and correctly bound to
.bb-view-pdf-dialog-modifiedso the populated value is visible.
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Potential null reference error if metadata.info.PDFFormatVersion is undefined. Add a null check or use optional chaining: version.textContent = metadata.info.PDFFormatVersion || ''
| version.textContent = metadata.info.PDFFormatVersion; | |
| version.textContent = metadata.info?.PDFFormatVersion || ''; |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Variable webview is declared but never used. It should be populated with fast web view information or removed if not needed.
| const webview = el.querySelector('.bb-view-pdf-dialog-webview'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): getFilesize can return undefined for large files and uses inconsistent rounding between units.
For metadata.contentLength >= 1024 * 1024 * 1024 * 1024, the function returns nothing, so the UI will display undefined. Also, MB/GB values are not rounded, unlike B/KB, which may produce long decimals. Please add a final fallback branch and standardize rounding across units (e.g. return ${(length / 1024 / 1024).toFixed(1)} MB;, and similarly for sizes ≥ 1 TB).
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The MB calculation should be rounded for consistency with KB formatting. Consider: return \${Math.round(length / 1024 / 1024)}MB``
| return `${length / 1024 / 1024}MB`; | |
| return `${Math.round(length / 1024 / 1024)}MB`; |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The GB calculation should be rounded for consistency with KB and MB formatting. Consider: return \${Math.round(length / 1024 / 1024 / 1024)}GB``
| return `${length / 1024 / 1024}MB`; | |
| } | |
| else if (length < 1024 * 1024 * 1024 * 1024) { | |
| return `${length / 1024 / 1024 / 1024}GB`; | |
| return `${Math.round(length / 1024 / 1024)}MB`; | |
| } | |
| else if (length < 1024 * 1024 * 1024 * 1024) { | |
| return `${Math.round(length / 1024 / 1024 / 1024)}GB`; |
Copilot
AI
Nov 29, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function doesn't have an explicit return statement for the GB case and beyond. Add an explicit return statement to ensure the function always returns a value, or add an else clause with a fallback.
| return `${length / 1024 / 1024}MB`; | |
| } | |
| else if (length < 1024 * 1024 * 1024 * 1024) { | |
| return `${length / 1024 / 1024 / 1024}GB`; | |
| } | |
| return `${Math.round(length / 1024 / 1024)}MB`; | |
| } | |
| else if (length < 1024 * 1024 * 1024 * 1024) { | |
| return `${Math.round(length / 1024 / 1024 / 1024)}GB`; | |
| } | |
| else { | |
| return `${Math.round(length / 1024 / 1024 / 1024 / 1024)}TB`; | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
issue (bug_risk): The same CSS class is used for the dialog header and the PDF title field, which breaks the JS metadata binding.
loadMetadatausesel.querySelector('.bb-view-pdf-dialog-title')and assumes it points to the Title metadata field, but in the markup that class is applied to both the header and the title value row. BecausequerySelectorreturns the first match (the header), the title cell never gets populated. Please give the title value a unique class (e.g..bb-view-pdf-dialog-title-value) and update the JS selector to match.