1.5.0-beta.12: Kotlin/Swift migration, 1.5.0 features, federated monorepo - #345
1.5.0-beta.12: Kotlin/Swift migration, 1.5.0 features, federated monorepo#345endigo wants to merge 43 commits into
Conversation
WalkthroughThe release migrates Android and iOS PDF platform views to Kotlin and Swift. It adds password unlocking, color modes, page alignment, tap callbacks, screenshots, rendering controls, integration tests, documentation, and version ChangesPDF view API and settings
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant FlutterApp
participant PDFViewController
participant NativePDFView
participant PDFKitOrAndroidPdfViewer
FlutterApp->>NativePDFView: create view with resolved settings
NativePDFView->>PDFKitOrAndroidPdfViewer: load PDF
PDFKitOrAndroidPdfViewer-->>NativePDFView: password or render callback
NativePDFView-->>PDFViewController: send method-channel event
PDFViewController-->>FlutterApp: invoke password or tap callback
FlutterApp->>PDFViewController: unlock with password
PDFViewController->>NativePDFView: invoke unlock
NativePDFView->>PDFKitOrAndroidPdfViewer: reload document
Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
3126c9a to
e1edb3f
Compare
|
Rebased onto the current
Verification after rebase: Android 🤖 Generated with Claude Code |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (7)
android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt (2)
474-477: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDeclare
linkHandlerasPDFLinkHandlerto remove the cast.The field at line 41 is typed
LinkHandler, but line 65 always assigns aPDFLinkHandler. Change the field type toPDFLinkHandlerand delete the downcast.♻️ Proposed change
- "preventLinkNavigation" -> { - val plh = this.linkHandler as PDFLinkHandler - plh.setPreventLinkNavigation(getBoolean(settings, key)) - } + "preventLinkNavigation" -> + linkHandler.setPreventLinkNavigation(getBoolean(settings, key))Apply this to the field declaration at line 41:
- private val linkHandler: LinkHandler + private val linkHandler: PDFLinkHandler🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt` around lines 474 - 477, Change the `linkHandler` field declaration in `FlutterPDFView` from `LinkHandler` to `PDFLinkHandler`, then remove the explicit cast in the `"preventLinkNavigation"` settings branch and call `setPreventLinkNavigation` directly on `linkHandler`.
79-83: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueUse a checked cast for
backgroundColor, likethumbnailRatio.
backgroundColor as NumberthrowsClassCastExceptionif Dart sends a non-numeric value. Lines 69-77 already use anis Numbercheck forthumbnailRatio. Apply the same pattern here for consistency.♻️ Proposed change
- val backgroundColor = params["backgroundColor"] - if (backgroundColor != null) { - val color = (backgroundColor as Number).toInt() - view.setBackgroundColor(color) - } + val backgroundColor = params["backgroundColor"] + if (backgroundColor is Number) { + view.setBackgroundColor(backgroundColor.toInt()) + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt` around lines 79 - 83, Update the backgroundColor handling in FlutterPDFView so it validates the value with an is Number check before converting it and calling view.setBackgroundColor, matching the existing thumbnailRatio pattern; avoid the unchecked cast that can throw ClassCastException for non-numeric values.android/build.gradle (1)
4-17: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRespect the host app’s Kotlin Gradle Plugin configuration.
This
classpathdeclaration may conflict with future Flutter/Flutter app Gradle scripts that declare KGP through declarative plugin DSL or a different version. Migrate to the recommended declarative Gradle Plugin DSL, or follow Flutter’s plugin author guidance for Kotlin Gradle Plugin handling.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/build.gradle` around lines 4 - 17, Update the root buildscript configuration around kotlin_version and the org.jetbrains.kotlin:kotlin-gradle-plugin classpath to avoid pinning or directly declaring the Kotlin Gradle Plugin, allowing the host Flutter app’s declarative plugin configuration and version to control it. Follow Flutter’s plugin author guidance while preserving the existing repository setup.ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift (4)
616-618: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winRemove the unused
FlutterPlatformViewconformance.
PDFViewControlleris the registered platform view, and itsview()returns theFlutterPDFViewcontainer. This secondview()returns the innerPDFViewinstead. Two differentFlutterPlatformViewimplementations return two different views, so a later change that passesFlutterPDFViewto Flutter would detach the container that performs the layout inlayoutSubviews.PDFViewControlleralso declaresPDFViewDelegateat Line 111 but never becomes the delegate; onlyFlutterPDFViewdoes.♻️ Proposed cleanup
-final class FlutterPDFView: UIView, FlutterPlatformView, PDFViewDelegate, +final class FlutterPDFView: UIView, PDFViewDelegate, UIGestureRecognizerDelegate, UIScrollViewDelegate- func view() -> UIView { - pdfView - } --final class PDFViewController: NSObject, FlutterPlatformView, PDFViewDelegate { +final class PDFViewController: NSObject, FlutterPlatformView {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 616 - 618, Remove the unused FlutterPlatformView conformance and its associated view() method from PDFViewController. Keep PDFViewController as the registered platform view returning the FlutterPDFView container, and preserve FlutterPDFView’s layout and PDFViewDelegate responsibilities.
622-624: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winReturn the live page count.
pageCountis assigned only inlayoutSubviews(Line 564). If Dart callspageCountfromonViewCreated, the first layout pass may not have run yet, so the handler returnsnil. The document is already available, so read the count from it.♻️ Proposed change
func getPageCount(_: FlutterMethodCall, result: FlutterResult) { - result(pageCount) + result(pageCount ?? (pdfView.document.map { NSNumber(value: $0.pageCount) })) }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 622 - 624, Update getPageCount to return the current page count directly from the loaded document rather than the cached pageCount property, while preserving the existing FlutterResult response contract.
821-832: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winRestrict the schemes that a PDF link can launch.
UIApplication.openaccepts any scheme that a registered app claims, not onlyhttpandhttps. A PDF from an untrusted source can therefore embedtel:,sms:,facetime:, or a third-party app scheme, and a tap starts that action. Limit the launch to web schemes, and let the DartonLinkHandlercallback handle everything else.🛡️ Proposed guard
func pdfViewWillClick(onLink _: PDFView, with url: URL) { - if !preventLinkNavigation { + let scheme = url.scheme?.lowercased() + if !preventLinkNavigation, scheme == "http" || scheme == "https" { UIApplication.shared.open(url, options: [:]) { success in🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 821 - 832, Update pdfViewWillClick so UIApplication.shared.open is invoked only when url.scheme is http or https (case-insensitively); leave other schemes exclusively to the existing onLinkHandler callback, preserving the preventLinkNavigation behavior for allowed web links.
518-531: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winConsider throttling
onDrawnotifications.The observation posts one
onDrawmethod-channel message for everycontentOffsetchange. During a scroll this can reach display-refresh frequency, so the Dart isolate receives up to 120 messages per second. The Android implementation throttles the equivalent callback withDRAW_THROTTLE_MS(seeandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt,onDraw). A timestamp guard restores parity and lowers channel traffic.♻️ Proposed throttle
+ /// Matches the Android `DRAW_THROTTLE_MS` guard. + private static let drawThrottle: TimeInterval = 0.016 + private var lastDrawTime: TimeInterval = 0 + private func startObserving() { guard let scrollView, contentOffsetObservation == nil else { return } contentOffsetObservation = scrollView.observe( \.contentOffset, options: [.new, .old] ) { [weak self] _, change in let newOffset = change.newValue ?? .zero let oldOffset = change.oldValue ?? .zero guard newOffset != oldOffset else { return } DispatchQueue.main.async { [weak self] in - self?.handleOnDraw() + guard let self else { return } + let now = CACurrentMediaTime() + guard now - lastDrawTime >= Self.drawThrottle else { return } + lastDrawTime = now + handleOnDraw() } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 518 - 531, Throttle onDraw notifications in startObserving by adding a timestamp guard equivalent to Android’s DRAW_THROTTLE_MS behavior. Track the last dispatched draw time and invoke handleOnDraw only when the throttle interval has elapsed, while preserving the existing content-offset change check and main-thread dispatch.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt`:
- Around line 412-423: Align setPage behavior across Android and iOS by choosing
a single out-of-range page contract and applying it consistently, preferably
validating the page index before invoking jumpTo and returning the same
INVALID_PAGE error response as iOS. In FlutterPDFView.setPage, parse the page
argument without assuming Int so decoded Long values do not cause
ClassCastException, then validate and preserve the agreed result behavior; if
clamping is intentionally retained, update the Dart API documentation and ensure
both platforms follow it.
- Around line 540-558: Replace unsafe platform-channel casts with safe casts
across the affected sites: in FlutterPDFView.kt lines 540-558, update
getBoolean, getString, and getInt to use as? Boolean, as? String, and
Number-to-int conversion; in FlutterPDFView.kt lines 79-83, guard
backgroundColor with an is Number check; in FlutterPDFView.kt lines 124-141, use
as? String and as? ByteArray so mismatches reach the existing null-config path;
and in PDFViewFactory.kt lines 14-17, safely cast args to the expected map and
fall back to emptyMap().
- Around line 540-558: Update getInt, getString, and getBoolean in
FlutterPDFView to use safe casts: convert numeric values through as? Number
followed by toInt(), cast strings with as? String, and cast booleans with as?
Boolean, preserving each helper’s absent-value defaults. Update
FlutterPDFViewParamsTest to assert the resulting defaults/null behavior instead
of expecting ClassCastException for wrong-type inputs.
In `@CHANGELOG.md`:
- Around line 5-8: Update the iOS changelog entry to distinguish the Swift
toolchain/manifest version from the CocoaPods source language mode: do not claim
migration to Swift 5.9 while ios/flutter_pdfview.podspec declares Swift 5.0.
Either revise the release note to match the existing podspec configuration or
update that configuration consistently before retaining the 5.9 wording.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 764-774: Update setZoomLimits to persist the requested minZoom and
maxZoom values in the stored minScaleFactor and maxScaleFactor properties used
by applyLayoutUpdates, rather than directly assigning computed PDFKit limits.
Remove the unsafe fitScale-based assignments from this handler so zero or NaN
fit scales cannot reach PDFKit, and let applyLayoutUpdates perform its existing
guarded calculation and application.
---
Nitpick comments:
In `@android/build.gradle`:
- Around line 4-17: Update the root buildscript configuration around
kotlin_version and the org.jetbrains.kotlin:kotlin-gradle-plugin classpath to
avoid pinning or directly declaring the Kotlin Gradle Plugin, allowing the host
Flutter app’s declarative plugin configuration and version to control it. Follow
Flutter’s plugin author guidance while preserving the existing repository setup.
In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt`:
- Around line 474-477: Change the `linkHandler` field declaration in
`FlutterPDFView` from `LinkHandler` to `PDFLinkHandler`, then remove the
explicit cast in the `"preventLinkNavigation"` settings branch and call
`setPreventLinkNavigation` directly on `linkHandler`.
- Around line 79-83: Update the backgroundColor handling in FlutterPDFView so it
validates the value with an is Number check before converting it and calling
view.setBackgroundColor, matching the existing thumbnailRatio pattern; avoid the
unchecked cast that can throw ClassCastException for non-numeric values.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 616-618: Remove the unused FlutterPlatformView conformance and its
associated view() method from PDFViewController. Keep PDFViewController as the
registered platform view returning the FlutterPDFView container, and preserve
FlutterPDFView’s layout and PDFViewDelegate responsibilities.
- Around line 622-624: Update getPageCount to return the current page count
directly from the loaded document rather than the cached pageCount property,
while preserving the existing FlutterResult response contract.
- Around line 821-832: Update pdfViewWillClick so UIApplication.shared.open is
invoked only when url.scheme is http or https (case-insensitively); leave other
schemes exclusively to the existing onLinkHandler callback, preserving the
preventLinkNavigation behavior for allowed web links.
- Around line 518-531: Throttle onDraw notifications in startObserving by adding
a timestamp guard equivalent to Android’s DRAW_THROTTLE_MS behavior. Track the
last dispatched draw time and invoke handleOnDraw only when the throttle
interval has elapsed, while preserving the existing content-offset change check
and main-thread dispatch.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 1bf4957d-1b14-4805-9b11-c0b6dd834c74
⛔ Files ignored due to path filters (1)
example/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (22)
CHANGELOG.mdandroid/build.gradleandroid/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFactory.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.javaandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFactory.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.ktexample/android/gradle.propertiesios/flutter_pdfview.podspecios/flutter_pdfview/Package.swiftios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.mios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swiftios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.mios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.swiftios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/FlutterPDFView.hios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/PDFViewFlutterPlugin.hios/flutter_pdfview/Sources/flutter_pdfview_objc/FPVExceptionCatcher.mios/flutter_pdfview/Sources/flutter_pdfview_objc/include/FPVExceptionCatcher.hpubspec.yaml
💤 Files with no reviewable changes (8)
- ios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.m
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.java
- ios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/PDFViewFlutterPlugin.h
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFactory.java
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.java
- ios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/FlutterPDFView.h
- android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
- ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m
Android (Java -> Kotlin, KGP 2.0.0, JVM target 17): - All four plugin classes converted; src/main is now pure Kotlin - Behavior-preserving port: param-getter semantics, link handling, and thumbnailRatio clamp verified by the existing 64 native unit tests running unchanged against the Kotlin sources iOS (Objective-C -> Swift 5.9): - Plugin, factory, controller, and platform view converted - Registered class name FLTPDFViewFlutterPlugin preserved via @objc; UIKit/PDFKit delegate selectors keep their ObjC names - NSException guards preserved through an Objective-C shim target (FPVExceptionCatcher) since Swift cannot catch NSException; SPM uses a dedicated target, CocoaPods compiles the mixed sources in one pod - KVO on scrollView contentOffset moved to block-based observation with explicit invalidation in deinit - Verified: flutter build ios (SPM path), pod lib lint dynamic and static, registrant symbol check via nm Dart: - Harden platform-view remount race: generation-tag creation callbacks so a late callback from a disposed view cannot complete the new controller Verified on final tree: flutter analyze clean, 76 plugin + 5 example Dart tests, 64 Android unit tests, example APK and iOS builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
The Objective-C fix (pdfURLFromFilePath) landed on the release branch after the Swift port was cut: URL(string:) returns nil for unescaped characters and falling back to fileURLWithPath on the full file:// string treats the scheme as part of the path. Replicate the helper in Swift, including file://localhost and file://hostname handling and percent-decoding. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
The migration branch was cut before 1819d87 landed on the release branch; its changes to the deleted Java/ObjC sources are re-applied to the ported implementations: - PDFLinkHandler.kt: only auto-launch http(s) links, add CATEGORY_BROWSABLE, catch RuntimeException so hostile file:// or intent:// links cannot crash the host app (FileUriExposedException is not an ActivityNotFoundException) - FlutterPDFView.kt: always recycle Pdfium via the main-thread handler on dispose (View.post is dropped on detached views and leaked Pdfium, #261); split "PDFView disposed" from "No pages loaded" in getCurrentPageSize - FlutterPDFView.swift: replace the single 0.1s scroll-config delay with a 5-attempt retry (PDFKit may expose its scroll view late) and report render completion on the exception path too Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
Re-applies b503410 (landed on main after the migration branch was cut) to the Swift implementation: fitPolicy (WIDTH/HEIGHT/BOTH) parity with Android, autoScales managed manually so spacing and zoom stay independent, re-fit after placeholder bounds and rotation preserving relative user zoom, rotation-aware fit-scale computation, and fit-state resets in reload/setZoomLimits/onDoubleTap. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
e1edb3f to
54d194e
Compare
Address CodeRabbit review on the migration PR: - setZoomLimits: validate arguments like Android (INVALID_ARGS for zero/inverted limits), and skip the immediate PDFKit application when the fit scale is still 0/NaN pre-layout — the persisted multipliers are applied by the next layout pass instead (prevents NaN reaching PDFKit, same class as #268) - podspec swift_version 5.0 -> 5.9 to match Package.swift and the changelog wording Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
Replace AndroidPdfViewer setNightMode with a ColorMatrixColorFilter on a hardware layer so dark mode preserves hue. PdfColorMatrix holds the involution matrix (null = light); applySettings records colorMode and backgroundColor then applies once; gutters use M(bg); screenshots use saveLayer so captures match the on-screen theme.
Adds proper dark/light theming to the iOS view and makes runtime setting updates take effect at all. FPVThemedPage is a PDFPage subclass installed unconditionally through PDFDocumentDelegate.classForPage(), so flipping the mode never has to re-instantiate pages. It reads the mode from the document delegate at draw time: light mode is a plain super.draw, dark mode renders super into an offscreen bitmap sized to the clip bounding box (PDFView tiles zoomed pages), prefilled white because PDFPage.draw does not clear its background, and runs the shared luminance-inverting matrix over it via CIColorMatrix. That matrix inverts lightness while preserving hue, so photos stay recognisable instead of becoming negatives, and it matches the Android constant. onUpdateSettings was a no-op stub that silently dropped all seven keys Dart sends. It now applies colorMode (plus the deprecated nightMode bool), backgroundColor, preventLinkNavigation, enableSwipe and min/maxZoom, and accepts-and-ignores pageFling/pageSnap and anything unknown rather than throwing. Note this changes behaviour for apps that relied on updates being dropped. A colorMode change re-renders through a position-preserving variant of reload(): PDFKit caches rendered pages with no cache-flush API, so the document is handed back to the view, then page/scale/scroll offset are restored and the zoom limits re-derived (reassigning the document resets PDFKit's min/max). The #150 fit state is deliberately left untouched so the next layout pass does not re-fit, and page-changed callbacks are suppressed across the swap. catchingNSException and the NSError helpers become internal so the new file can guard its PDFKit calls the same way. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01X4UXNB2fV5BC8UUqzvBq6Q
Expose colorMode (light/dark/system) on PDFView, resolve system from Theme brightness, deprecate nightMode, push colorMode and backgroundColor in updatesMap, and update example/docs/tests accordingly.
# Conflicts: # CHANGELOG.md # example/pubspec.lock # pubspec.yaml
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
1.5.0-beta.1 publishedPushed two commits and published the beta:
Gates re-run on the merged state
Per the release policy, target full |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.kt (1)
18-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winReplace the empty lifecycle override with an explicit no-op.
Detekt reports
EmptyFunctionBlockforonDetachedFromEngine. If no engine-scoped resource requires cleanup, use an expression body to preserve the no-op and remove the warning. IfPDFViewFactoryowns such resources, release them here instead.Proposed change
- override fun onDetachedFromEngine(binding: FlutterPlugin.FlutterPluginBinding) { - } + override fun onDetachedFromEngine(binding: FlutterPlugin.FlutterPluginBinding) = Unit🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.kt` around lines 18 - 19, Update the PDFViewFlutterPlugin.onDetachedFromEngine override to use an explicit expression-body no-op when no engine-scoped cleanup is needed, eliminating the EmptyFunctionBlock warning; if PDFViewFactory owns engine-scoped resources, release them in this lifecycle method instead.Source: Linters/SAST tools
ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift (1)
540-553: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winThrottle
onDrawto match the Android implementation.The observation posts
handleOnDrawfor everycontentOffsetchange. Each call sends a method-channel message. During a scroll, this produces one message per frame or more. The Android implementation throttles the same callback withDRAW_THROTTLE_MS(seeandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt). Add an equivalent time-based throttle so both platforms send comparable traffic.♻️ Proposed throttle
+ /// Matches Android's DRAW_THROTTLE_MS so both platforms emit onDraw at a + /// comparable rate. + private static let drawThrottle: TimeInterval = 1.0 / 60.0 + private var lastDrawTime: TimeInterval = 0 + private func startObserving() { guard let scrollView, contentOffsetObservation == nil else { return } contentOffsetObservation = scrollView.observe( \.contentOffset, options: [.new, .old] ) { [weak self] _, change in let newOffset = change.newValue ?? .zero let oldOffset = change.oldValue ?? .zero guard newOffset != oldOffset else { return } DispatchQueue.main.async { [weak self] in - self?.handleOnDraw() + guard let self else { return } + let now = CACurrentMediaTime() + guard now - lastDrawTime >= Self.drawThrottle else { return } + lastDrawTime = now + handleOnDraw() } } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 540 - 553, Update startObserving and the handleOnDraw dispatch path to apply an Android-equivalent time-based throttle using DRAW_THROTTLE_MS (or the corresponding iOS duration), ensuring contentOffset changes within the throttle window do not send additional onDraw method-channel messages while preserving the existing callback behavior after the interval.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 974-985: Update pdfViewWillClick(onLink:with:) so
UIApplication.shared.open is called only when the URL scheme is HTTP or HTTPS,
using a case-insensitive check. Preserve invoking
controller?.invokeChannelMethod("onLinkHandler", arguments: url.absoluteString)
for every link, regardless of scheme.
- Around line 839-863: The reload flow in reload() must reset didLoadComplete
and hasSentInitialPage before reconfiguring the PDF view, then invoke
handleRenderCompleted(document.pageCount) after the replacement document is
configured so the new document emits its load-completion callback. Preserve the
existing reload result behavior.
In `@README.md`:
- Line 26: Update the dependency code fence in README.md to use the yaml
language identifier, changing the opening fence from ``` to ```yaml while
preserving the block contents.
- Line 21: Update the “Trying the 1.5.0 beta” heading from level 4 to level 3 so
it follows the preceding level-2 heading and preserves the README heading
hierarchy.
---
Nitpick comments:
In
`@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.kt`:
- Around line 18-19: Update the PDFViewFlutterPlugin.onDetachedFromEngine
override to use an explicit expression-body no-op when no engine-scoped cleanup
is needed, eliminating the EmptyFunctionBlock warning; if PDFViewFactory owns
engine-scoped resources, release them in this lifecycle method instead.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 540-553: Update startObserving and the handleOnDraw dispatch path
to apply an Android-equivalent time-based throttle using DRAW_THROTTLE_MS (or
the corresponding iOS duration), ensuring contentOffset changes within the
throttle window do not send additional onDraw method-channel messages while
preserving the existing callback behavior after the interval.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 34316dc3-7220-4f9a-920a-639c38191e6e
⛔ Files ignored due to path filters (1)
example/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (23)
CHANGELOG.mdREADME.mdandroid/build.gradleandroid/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFactory.javaandroid/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.javaandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFactory.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.ktexample/android/gradle.propertiesios/flutter_pdfview.podspecios/flutter_pdfview/Package.swiftios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.mios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swiftios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.mios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.swiftios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/FlutterPDFView.hios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/PDFViewFlutterPlugin.hios/flutter_pdfview/Sources/flutter_pdfview_objc/FPVExceptionCatcher.mios/flutter_pdfview/Sources/flutter_pdfview_objc/include/FPVExceptionCatcher.hpubspec.yaml
💤 Files with no reviewable changes (8)
- ios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.m
- ios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/PDFViewFlutterPlugin.h
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFactory.java
- ios/flutter_pdfview/Sources/flutter_pdfview/include/flutter_pdfview/FlutterPDFView.h
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFViewFlutterPlugin.java
- ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.m
- android/src/main/java/io/endigo/plugins/pdfviewflutter/FlutterPDFView.java
- android/src/main/java/io/endigo/plugins/pdfviewflutter/PDFLinkHandler.java
🚧 Files skipped from review as they are similar to previous changes (9)
- example/android/gradle.properties
- ios/flutter_pdfview.podspec
- ios/flutter_pdfview/Sources/flutter_pdfview/PDFViewFlutterPlugin.swift
- pubspec.yaml
- ios/flutter_pdfview/Sources/flutter_pdfview_objc/include/FPVExceptionCatcher.h
- CHANGELOG.md
- android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PDFViewFactory.kt
- ios/flutter_pdfview/Package.swift
- android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt
| func reload(_: FlutterMethodCall, result: FlutterResult) { | ||
| pdfView.document = document | ||
| hasAppliedInitialFit = false | ||
| lastFitScale = 0 | ||
| lastLayoutSize = .zero | ||
| if let document, document.pageCount > 0, let firstPage = document.page(at: 0) { | ||
| pdfView.go(to: firstPage) | ||
|
|
||
| let pageBounds = firstPage.bounds(for: .mediaBox) | ||
| pdfView.go( | ||
| to: CGRect(x: 0, y: pageBounds.size.height, width: 1, height: 1), | ||
| on: firstPage | ||
| ) | ||
|
|
||
| let fitScale = fitScaleForCurrentPolicy() | ||
| if fitScale.isFinite, fitScale > 0 { | ||
| pdfView.scaleFactor = fitScale | ||
| lastFitScale = fitScale | ||
| lastLayoutSize = bounds.size | ||
| hasAppliedInitialFit = true | ||
| } | ||
| } | ||
|
|
||
| result(NSNumber(value: true)) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Compare reload behaviour and render callbacks across platforms and Dart.
fd -e kt -e dart -e swift | xargs rg -n -C6 '\breload\b|onLoadComplete|onRender'Repository: endigo/flutter_pdfview
Length of output: 36787
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "iOS FlutterPDFView.swift relevant sections"
sed -n '120,200p' ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift
sed -n '800,970p' ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift
echo
echo "Android FlutterPDFView.kt relevant sections"
sed -n '130,205p' android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt
sed -n '382,400p' android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt
echo
echo "Dart reload and callback handling"
sed -n '186,192p' lib/src/pdf_view_controller.dart
sed -n '50,70p' lib/src/pdf_view_controller.dart
echo
echo "Find document loading / initial render triggers in iOS"
rg -n -C 5 'loadDocument|handleRenderCompleted|didLoadComplete|hasSentInitialPage|invokeChannelMethod\("onRender"|onLoadComplete' ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swiftRepository: endigo/flutter_pdfview
Length of output: 18296
Reload should reset completion state before reconfiguring the iOS PDF view.
reload() replaces the document but leaves didLoadComplete true and hasSentInitialPage true. handleRenderCompleted() emits onLoadComplete only while didLoadComplete is false, so reload callbacks can miss the new document state. Reset these flags in reload() and call handleRenderCompleted(document.pageCount) when the reload completes, matching Android’s callback contract.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around
lines 839 - 863, The reload flow in reload() must reset didLoadComplete and
hasSentInitialPage before reconfiguring the PDF view, then invoke
handleRenderCompleted(document.pageCount) after the replacement document is
configured so the new document emits its load-completion callback. Preserve the
existing reload result behavior.
| `1.5.0-beta.1` ports the native implementations to Kotlin (Android) and Swift (iOS) with no | ||
| public Dart API changes. Pre-releases are not picked up by a `^` constraint, so pin it explicitly: | ||
|
|
||
| ``` |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add a language identifier to the dependency code fence.
Use ```yaml instead of ``` so the fenced block identifies its syntax and resolves MD040.
🧰 Tools
🪛 markdownlint-cli2 (0.23.1)
[warning] 26-26: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 26, Update the dependency code fence in README.md to use
the yaml language identifier, changing the opening fence from ``` to ```yaml
while preserving the block contents.
Source: Linters/SAST tools
…213) ColorFiltered and ShaderMask do not apply to UiKitView contents; this is a Flutter composition limitation, not a plugin bug. Document official evidence and practical workarounds (nightMode, scrim/blur layout, screenshot + filter).
gestureRecognizers + TapGestureRecognizer is unreliable on platform views. Report single taps from native AndroidPdfViewer / PDFKit via onTap instead.
Pdfium paints widget appearance streams and does not regenerate broken/missing appearances like Adobe. Document producer-side workarounds after investigating the sample PDF's first PDTextField.
Defer document open until the platform view has a usable non-zero size, keep the native view hidden until first successful fit/render, and re-fit when the first layout settles. iOS open paths now report missing, unreadable, empty, or corrupt documents via onError and force a layout pass after attach so PDFs are not stuck blank until background/foreground. Addresses #40, #127, #190. No package version bump (coordinator release).
Hybrid composition and hardware layers made View.draw / drawing-cache screenshots blank white. Android now prefers PixelCopy of the platform view with a software-layer draw fallback; iOS implements getScreenshot by rasterizing the PDFKit layer and falling back to PDFPage.draw.
Android: density-aware page-part cache, apply Dart thumbnailRatio 0.8 when omitted (library default 0.3), keep useBestQuality / enableAntialiasing / enableRenderDuringScale true when params missing. iOS: pin PDFView contentScaleFactor to screen scale. Document quality knobs and hard Pdfium spatial-resolution limits in README.
An encrypted PDF used to leave a blank view with no way to supply a password: Android surfaced Pdfium's failure only as an opaque onError string, and iOS skipped the setup a document needs to render, so even a correct password could not recover the view. - New `onPasswordRequired` callback reporting whether the document needs a password (`PDFPasswordFailure.missing`) or rejected the one it was given (`PDFPasswordFailure.incorrect`) - New `PDFViewController.unlock(password)`, and `password` is now part of the settings diff, so both the imperative and the declarative route reopen the document inside the existing platform view — a wrong password can be retried without recreating the viewer - Android: recognise `PdfPasswordException` (including wrapped and repackaged variants) and re-run the configurator with the new password, completing `unlock` from the load callbacks - iOS: defer the page/scroll setup while the document is locked and run it once a password opens it; `getPageCount()` no longer returns null between the render callback and the first layout pass Tests: Dart unit coverage for the channel contract, Android unit coverage for the exception detection, and an integration_test suite driving the real native viewers against a checked-in encrypted fixture (generated by scripts/make_protected_pdf.py). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
feat: unlock password-protected documents (#274)
Unlock password-protected documents (#274) after merge of #359. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
f83bdfc to
c1e9c27
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift (2)
1715-1726: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRestrict automatic link launching to HTTP(S).
pdfViewWillClick(onLink:with:)still opens every URL the document contains. A PDF is untrusted input, so a crafted document can triggertel:,sms:,mailto:, or a custom app scheme with no user confirmation.PDFLinkHandler.kton Android limits automatic launching to HTTP(S), so the platforms diverge.A previous review flagged this and the thread is marked as addressed, but the reviewed code contains no scheme check.
🔒️ Proposed fix
func pdfViewWillClick(onLink _: PDFView, with url: URL) { - if !preventLinkNavigation { + let scheme = url.scheme?.lowercased() + let isWebLink = scheme == "http" || scheme == "https" + if !preventLinkNavigation, isWebLink { UIApplication.shared.open(url, options: [:]) { success in🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 1715 - 1726, Update pdfViewWillClick(onLink:with:) so automatic UIApplication.shared.open execution occurs only when url.scheme is HTTP or HTTPS, using a case-insensitive comparison; continue invoking onLinkHandler for every clicked URL.
1154-1189: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
reloaddoes not reset the completion flags and never reports render completion.
reloadreplaces the document and resets the fit state, but it leavesdidLoadCompleteandhasSentInitialPageset totrue, and it does not callhandleRenderCompleted. Dart therefore receives noonRenderand noonLoadCompleteafterPDFViewController.reload(). Android'sreloadre-runs the configurator, so both callbacks fire there.Reset the flags and report completion so both platforms deliver the same callbacks.
🐛 Proposed fix
pdfView.document = document hasAppliedInitialFit = false lastFitScale = 0 lastLayoutSize = .zero isContentRevealed = false pdfView.isHidden = true + didLoadComplete = false + hasSentInitialPage = false + defaultPageSet = false if let document, document.pageCount > 0, let firstPage = document.page(at: 0) {Then report completion before returning the result:
if let document { handleRenderCompleted(NSNumber(value: document.pageCount)) } result(NSNumber(value: true))🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around lines 1154 - 1189, Update reload to reset didLoadComplete and hasSentInitialPage when replacing the document, then invoke handleRenderCompleted with the reloaded document’s page count before returning the successful result. Preserve the existing deferred-load path and fit/reset behavior.
🧹 Nitpick comments (4)
ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift (1)
39-47: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winUse exact fractions so the matrix matches Android and stays an involution.
The Android constant uses
1f/3fand-2f/3f. Here the coefficients are rounded to0.333and-0.667, so each row sums to-1.001instead of-1. White maps to-0.001and clamps to 0, andM(M(v))drifts by about 0.1% per pass. The doc comment above claims the matrix is shared with Android and is an involution.♻️ Proposed fix
- private static let diagonal: CGFloat = 0.333 - private static let offDiagonal: CGFloat = -0.667 + private static let diagonal: CGFloat = 1.0 / 3.0 + private static let offDiagonal: CGFloat = -2.0 / 3.0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift` around lines 39 - 47, Update the diagonal and offDiagonal constants used by luminanceInvertR, luminanceInvertG, and luminanceInvertB to use exact one-third and negative two-thirds fractions, matching the Android constants and preserving the matrix’s involution property; leave the alpha and bias vectors unchanged.android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt (2)
858-921: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
centeredSecondaryOffsetanddocumentFitsAlongPrimaryboth scan every page.Each call iterates
pageCountand callsview.getPageSize(i).applyPagePlacementruns after load, after everysetPage, and after eachpageAlignmentupdate, so a large document performs two full scans per navigation on the main thread.Cache the maximum secondary dimension and the total primary length per document and per zoom level, and invalidate the cache on reload or zoom change.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt` around lines 858 - 921, Cache the computed maximum secondary dimension and total primary document length used by centeredSecondaryOffset and documentFitsAlongPrimary, keyed by the current document and zoom level, so applyPagePlacement avoids rescanning every page on navigation. Reuse cached values in both helpers and invalidate the cache whenever the document reloads or the zoom changes.
884-898: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winReflection on
PDFView.currentYOffsetcan silently disable top alignment in release builds.
forcePrimaryOffsetIfShortresolves the field by name throughgetDeclaredField. If R8 renames or removes that field in the app's release build, the lookup throws, thecatchlogs a warning, andPageAlignment.topsilently falls back to centered layout. The failure is invisible to the Dart API, so users see a behavior difference between debug and release.Add a consumer ProGuard rule that keeps these fields, or replace the reflection with a public API call such as
moveTocombined with the offsets computed fromdocumentFitsAlongPrimary.Also confirm the field names exist in AndroidPdfViewer 3.2.8 and are not obfuscated by the plugin's own consumer rules.
#!/bin/bash # Check for consumer ProGuard rules that protect the reflected AndroidPdfViewer fields. fd -t f -e pro -e txt . android | while IFS= read -r f; do echo "=== $f ==="; cat "$f" done rg -n 'consumerProguardFiles|minifyEnabled|proguard' --glob '*.gradle' --glob '*.gradle.kts' rg -n 'currentXOffset|currentYOffset|getDeclaredField' -g '*.kt' -g '*.java'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt` around lines 884 - 898, Protect the reflected AndroidPdfViewer fields used by forcePrimaryOffsetIfShort with a consumer ProGuard rule, covering currentXOffset and currentYOffset and preserving their names for AndroidPdfViewer 3.2.8. Verify the fields exist in that dependency and that the rule is included by the plugin so release builds retain the top-alignment behavior.test/page_alignment_test.dart (1)
19-67: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicated platform-view mock harness in
test/page_alignment_test.dartandtest/password_test.dart. Both new test files copy the sameSystemChannels.platform_viewscreatehandler, creation-params decoding, per-view channel recording, and teardown loop that already exists intest/creation_params_test.dart. One shared helper removes the need to keep three copies in sync.
test/page_alignment_test.dart#L19-L67: replace the localsetUp/tearDownwith the shared harness.test/password_test.dart#L157-L205: replace the localsetUp/tearDownin thepassword changes over the method channelgroup with the same shared harness.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/page_alignment_test.dart` around lines 19 - 67, Extract the duplicated platform-view mock setup, parameter decoding, per-view channel recording, and teardown into one shared test helper, following the existing harness in test/creation_params_test.dart. Replace the local setUp/tearDown blocks in test/page_alignment_test.dart lines 19-67 and test/password_test.dart lines 157-205 with that helper; both sites require the same direct change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@example/integration_test/password_test.dart`:
- Around line 185-188: Wrap the awaited reported.controller!.getPageCount() call
in tester.runAsync, matching the existing unlock and setPage patterns. Apply the
same wrapping to the corresponding controller calls around the other affected
assertions in this test, including the cases near the later referenced
locations, while preserving their existing expectations.
In `@example/lib/main.dart`:
- Around line 282-338: Update _promptForPassword to create the
TextEditingController before calling showDialog instead of inside its builder,
and dispose it in a finally block that wraps the dialog interaction. Preserve
the existing cancel, submit, and dismissal behavior while ensuring disposal
occurs on every exit path.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 1233-1235: Update the password handling in FlutterPDFView’s
settings application to recognize an NSNull value from
_PDFViewSettings.updatesMap as a cleared password and invoke applyPassword
accordingly, matching Android’s document-reopen behavior. Preserve the existing
handling for non-null String passwords.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift`:
- Around line 85-89: Synchronize access to isDarkMode between setColorMode and
FPVThemedPage.draw(with:to:), using the existing FlutterPDFView state-management
pattern or an os_unfair_lock/atomic wrapper. Ensure draw reads a consistent
snapshot while main-thread updates during loadDocument and onUpdateSettings
remain safe.
---
Duplicate comments:
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift`:
- Around line 1715-1726: Update pdfViewWillClick(onLink:with:) so automatic
UIApplication.shared.open execution occurs only when url.scheme is HTTP or
HTTPS, using a case-insensitive comparison; continue invoking onLinkHandler for
every clicked URL.
- Around line 1154-1189: Update reload to reset didLoadComplete and
hasSentInitialPage when replacing the document, then invoke
handleRenderCompleted with the reloaded document’s page count before returning
the successful result. Preserve the existing deferred-load path and fit/reset
behavior.
---
Nitpick comments:
In `@android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt`:
- Around line 858-921: Cache the computed maximum secondary dimension and total
primary document length used by centeredSecondaryOffset and
documentFitsAlongPrimary, keyed by the current document and zoom level, so
applyPagePlacement avoids rescanning every page on navigation. Reuse cached
values in both helpers and invalidate the cache whenever the document reloads or
the zoom changes.
- Around line 884-898: Protect the reflected AndroidPdfViewer fields used by
forcePrimaryOffsetIfShort with a consumer ProGuard rule, covering currentXOffset
and currentYOffset and preserving their names for AndroidPdfViewer 3.2.8. Verify
the fields exist in that dependency and that the rule is included by the plugin
so release builds retain the top-alignment behavior.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift`:
- Around line 39-47: Update the diagonal and offDiagonal constants used by
luminanceInvertR, luminanceInvertG, and luminanceInvertB to use exact one-third
and negative two-thirds fractions, matching the Android constants and preserving
the matrix’s involution property; leave the alpha and bias vectors unchanged.
In `@test/page_alignment_test.dart`:
- Around line 19-67: Extract the duplicated platform-view mock setup, parameter
decoding, per-view channel recording, and teardown into one shared test helper,
following the existing harness in test/creation_params_test.dart. Replace the
local setUp/tearDown blocks in test/page_alignment_test.dart lines 19-67 and
test/password_test.dart lines 157-205 with that helper; both sites require the
same direct change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: be1ca432-00e8-4309-bf82-0e33492fe083
⛔ Files ignored due to path filters (2)
example/assets/demo-protected.pdfis excluded by!**/*.pdfexample/pubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
CHANGELOG.mdREADME.mdandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.ktandroid/src/main/kotlin/io/endigo/plugins/pdfviewflutter/PdfColorMatrix.ktandroid/src/test/java/com/example/pdfiumfork/PdfPasswordException.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewColorModeTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewParamsTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewPasswordTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewQualityDefaultsTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewScreenshotTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/FlutterPDFViewThumbnailRatioTest.javaandroid/src/test/java/io/endigo/plugins/pdfviewflutter/PdfColorMatrixTest.javaexample/integration_test/password_test.dartexample/lib/main.dartexample/pubspec.yamlexample/test/widget_test.dartios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swiftios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swiftlib/flutter_pdfview.dartlib/src/pdf_view.dartlib/src/pdf_view_controller.dartlib/src/pdf_view_settings.dartlib/src/types.dartpubspec.yamlscripts/make_protected_pdf.pytest/creation_params_test.darttest/flutter_pdfview_test.darttest/page_alignment_test.darttest/password_test.darttest/pdf_view_controller_test.dart
🚧 Files skipped from review as they are similar to previous changes (1)
- pubspec.yaml
| expect(await waitFor(tester, () => reported.loadedPages != null), isTrue); | ||
| expect(reported.passwordFailures, isEmpty); | ||
| expect(await reported.controller!.getPageCount(), protectedPageCount); | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Wrap the controller calls in tester.runAsync for consistency.
Line 187 awaits getPageCount() inside the fake-async test zone. The other platform-channel calls in this file (unlock, setPage) run inside tester.runAsync. A native reply that needs real asynchronous time can stall in the test zone and time out. The same pattern appears at Line 244 and Line 323.
♻️ Proposed change
- expect(await reported.controller!.getPageCount(), protectedPageCount);
+ late int? pageCount;
+ await tester.runAsync(() async {
+ pageCount = await reported.controller!.getPageCount();
+ });
+ expect(pageCount, protectedPageCount);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| expect(await waitFor(tester, () => reported.loadedPages != null), isTrue); | |
| expect(reported.passwordFailures, isEmpty); | |
| expect(await reported.controller!.getPageCount(), protectedPageCount); | |
| expect(await waitFor(tester, () => reported.loadedPages != null), isTrue); | |
| expect(reported.passwordFailures, isEmpty); | |
| late int? pageCount; | |
| await tester.runAsync(() async { | |
| pageCount = await reported.controller!.getPageCount(); | |
| }); | |
| expect(pageCount, protectedPageCount); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/integration_test/password_test.dart` around lines 185 - 188, Wrap the
awaited reported.controller!.getPageCount() call in tester.runAsync, matching
the existing unlock and setPage patterns. Apply the same wrapping to the
corresponding controller calls around the other affected assertions in this
test, including the cases near the later referenced locations, while preserving
their existing expectations.
| /// Guards against stacking dialogs: a rejected password reports again. | ||
| bool _isPrompting = false; | ||
|
|
||
| /// Asks for a password and hands it to the controller, which reopens the | ||
| /// document in place. | ||
| Future<void> _promptForPassword(PDFPasswordFailure failure) async { | ||
| if (_isPrompting) { | ||
| return; | ||
| } | ||
| _isPrompting = true; | ||
| // The prompt replaces the error banner for this failure. | ||
| setState(() { | ||
| _errorMessage = ''; | ||
| }); | ||
| try { | ||
| final PDFViewController controller = await _controller.future; | ||
| if (!mounted) { | ||
| return; | ||
| } | ||
| final String? password = await showDialog<String>( | ||
| context: context, | ||
| barrierDismissible: false, | ||
| builder: (BuildContext context) { | ||
| final TextEditingController field = TextEditingController(); | ||
| return AlertDialog( | ||
| title: const Text('Password required'), | ||
| content: TextField( | ||
| controller: field, | ||
| autofocus: true, | ||
| obscureText: true, | ||
| decoration: InputDecoration( | ||
| labelText: 'Password', | ||
| errorText: failure == PDFPasswordFailure.incorrect | ||
| ? 'That password did not open the document' | ||
| : null, | ||
| ), | ||
| onSubmitted: (String value) => Navigator.pop(context, value), | ||
| ), | ||
| actions: <Widget>[ | ||
| TextButton(onPressed: () => Navigator.pop(context), child: const Text('Cancel')), | ||
| TextButton( | ||
| onPressed: () => Navigator.pop(context, field.text), | ||
| child: const Text('Open'), | ||
| ), | ||
| ], | ||
| ); | ||
| }, | ||
| ); | ||
| if (password == null || !mounted) { | ||
| return; | ||
| } | ||
| final bool unlocked = await controller.unlock(password); | ||
| debugPrint(unlocked ? 'document unlocked' : 'wrong password'); | ||
| } finally { | ||
| _isPrompting = false; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Dispose the TextEditingController created for the password dialog.
The field controller at line 305 is created inside the showDialog builder. Nothing disposes it, whichever path the dialog takes: cancel, submit, or dismiss. This leaks a controller on every password prompt. Move field creation before the showDialog call and dispose it in a finally block so it is guaranteed to run.
🧹 Proposed fix to dispose the controller
try {
final PDFViewController controller = await _controller.future;
if (!mounted) {
return;
}
+ final TextEditingController field = TextEditingController();
final String? password = await showDialog<String>(
context: context,
barrierDismissible: false,
builder: (BuildContext context) {
- final TextEditingController field = TextEditingController();
return AlertDialog(
title: const Text('Password required'),
content: TextField(
controller: field,
autofocus: true,
obscureText: true,
decoration: InputDecoration(
labelText: 'Password',
errorText: failure == PDFPasswordFailure.incorrect
? 'That password did not open the document'
: null,
),
onSubmitted: (String value) => Navigator.pop(context, value),
),
actions: <Widget>[
TextButton(onPressed: () => Navigator.pop(context), child: const Text('Cancel')),
TextButton(
onPressed: () => Navigator.pop(context, field.text),
child: const Text('Open'),
),
],
);
},
);
+ field.dispose();
if (password == null || !mounted) {
return;
}
final bool unlocked = await controller.unlock(password);
debugPrint(unlocked ? 'document unlocked' : 'wrong password');
} finally {
_isPrompting = false;
}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@example/lib/main.dart` around lines 282 - 338, Update _promptForPassword to
create the TextEditingController before calling showDialog instead of inside its
builder, and dispose it in a finally block that wraps the dialog interaction.
Preserve the existing cancel, submit, and dismissal behavior while ensuring
disposal occurs on every exit path.
| if let password = settings["password"] as? String { | ||
| applyPassword(password) | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
A cleared password is ignored on iOS but reopens the document on Android.
settings["password"] as? String fails for NSNull, so clearing PDFView.password at runtime is a no-op here. Dart emits updates['password'] = null in _PDFViewSettings.updatesMap, and Android's applySettings calls applyPassword(null, null), which recycles and reopens the document without a password.
Either handle the NSNull case the same way, or state the platform difference in the PDFView.password doc comment, which currently says only that changing it reopens the document.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift` around
lines 1233 - 1235, Update the password handling in FlutterPDFView’s settings
application to recognize an NSNull value from _PDFViewSettings.updatesMap as a
cleared password and invoke applyPassword accordingly, matching Android’s
document-reopen behavior. Preserve the existing handling for non-null String
passwords.
| override func draw(with box: PDFDisplayBox, to context: CGContext) { | ||
| guard let owner = document?.delegate as? FlutterPDFView, owner.isDarkMode else { | ||
| super.draw(with: box, to: context) | ||
| return | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
isDarkMode is read off the main thread without synchronization.
draw(with:to:) reads owner.isDarkMode. The comment on isDarkMode in FlutterPDFView.swift states that PDFKit renders pages off the main thread, while setColorMode writes the property from the main thread during loadDocument and onUpdateSettings. That is an unsynchronized cross-thread access to a mutable Bool.
The visible effect is limited, because rerenderPreservingPosition() re-renders after a mode flip. Still, make the access explicit: guard the property with an os_unfair_lock or an atomic wrapper, or snapshot the mode into a let that only the main thread replaces alongside the document swap.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift` around lines
85 - 89, Synchronize access to isDarkMode between setColorMode and
FPVThemedPage.draw(with:to:), using the existing FlutterPDFView state-management
pattern or an os_unfair_lock/atomic wrapper. Ensure draw reads a consistent
snapshot while main-thread updates during loadDocument and onUpdateSettings
remain safe.
Move the published package to packages/flutter_pdfview/ and turn the repository root into a Dart pub workspace driven by Melos 7. Nothing inside the package changes; this is layout only, so that platform implementations and a future web package can be split out without another repository move. - Root pubspec.yaml declares the workspace members and inlines the melos config (analyze / format / test / test:android / publish:dry-run scripts) - Members carry `resolution: workspace`; the single lockfile lives at the root and member lockfiles are untracked and gitignored - .gitignore patterns that were anchored to the old root (ios/.symlinks, example/android/app/.cxx, ...) are matched at any depth instead - publish.yml sets working-directory: packages/flutter_pdfview - scripts/ repointed at the new package location - CLAUDE.md documents the layout and drops paths that were stale from both this move and the earlier Kotlin/Swift migration (it still said .java/.m) Verified: 127 Dart tests pass, the Android Robolectric/JUnit suite builds green through the repointed script, `melos run analyze` and `format-check` are clean, and `flutter pub publish --dry-run` reports no content problems. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
Split the plugin into a federated structure. The shared value types, the settings/creation-param wire format and the method-channel plumbing move into a new flutter_pdfview_platform_interface package; PDFView now reaches the native views through FlutterPdfViewPlatform.instance. Adding a platform (web, macOS, Windows) no longer means touching the app-facing package — an implementation extends FlutterPdfViewPlatform and registers itself. What moved: - FitPolicy / PageAlignment / PdfColorMode / PDFPasswordFailure and every callback typedef, re-exported from flutter_pdfview so the public import path is unchanged - PdfViewSettings (was the private _PDFViewSettings), including updatesMap — the diff that updates a live view instead of remounting it - PdfViewCreationParams (was _CreationParams) - The platform-view construction: UiKitView, PlatformViewLink + initExpensiveAndroidView, and the unsupported-platform Text fallback - The per-view method channel, as MethodChannelPdfViewController What did not change: - The public Dart API. PDFView, PDFViewController, every callback and enum keep the same names, semantics and import path - The native code, the platform-view type and the method-channel protocol, so existing native implementations keep working PDFViewController is now a thin wrapper over PdfViewPlatformController. Its callbacks are a PdfViewCallbacks snapshot refreshed on every widget update rather than a live read of the widget, so the controller re-points them at construction and on each update — preserving the previous behavior that a rebuilt widget's callbacks take effect. Verified: the full 1.5.0-beta.10 Dart suite (127 tests) passes unmodified, plus 38 new tests for the interface covering the wire format, the PlatformInterface token check and the method-channel controller. Android Robolectric/JUnit green, analyze and format clean, and both packages pass `pub publish --dry-run` (the interface with zero warnings). Release order: flutter_pdfview_platform_interface must be published before flutter_pdfview, which depends on it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
`melos bootstrap` writes a melos_<package>.iml next to each workspace member and one at the root. They are per-machine IDE files, not project config. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
…tlin
Two warnings that Flutter 3.44 prints on every build of an app using this
plugin. Neither was caused by the monorepo/federation work; both are the
plugin lagging behind toolchain changes.
iOS — Package.swift declared `dependencies: []`. Flutter 3.44+ requires
plugins to depend on FlutterFramework explicitly rather than inheriting the
embedder from the generated Runner package. Declared it and added the product
to the Swift target. The Objective-C shim only imports Foundation, so it
deliberately does not take the dependency.
Android — AGP 9 compiles Kotlin itself, and Flutter fails the build of any app
whose plugins also apply the Kotlin Gradle Plugin, so this plugin would have
broken apps on future Flutter releases. Applying KGP unconditionally causes
that; removing it unconditionally would break everyone on AGP 8, which this
plugin still supports (pubspec requires only Flutter >= 3.32). So it is now
applied only when AGP < 9, and the `kotlinOptions` block is replaced by
`kotlin { compilerOptions { jvmTarget } }`, which both toolchains understand.
Verified on the example app by temporarily moving it to AGP 9.3.1 + Gradle
9.5.0 + android.builtInKotlin=true: flutter_pdfview evaluates cleanly and
appears in no failure. The build does still fail there, but on packages
outside this repo that have not migrated — Flutter's own integration_test and
jni. The example app config was restored afterwards and is unchanged.
The KGP warning does not disappear on AGP 8, because the plugin still applies
KGP there by design. It goes away once an app is on AGP 9 with built-in Kotlin,
which is the case this change exists to keep working.
Also ignore SwiftPM .build/ output, which `pod lib lint` leaves behind.
Verified after the change: iOS builds with no FlutterFramework warning, iOS
integration tests 11/11 against real PDFKit, Android APK builds, Android unit
tests 123/0/0, Dart 127 + 38 pass, analyze clean, and `pod lib lint` still
passes so the CocoaPods path is unaffected.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XKmmG2v9eJSWFDLy6mRe8F
refactor: federated plugin — Melos monorepo + flutter_pdfview_platform_interface
endigo
changed the title
Reimplement long-standing community PRs on the federated stack: - setPage(withAnimation:) for Android jumpTo animation (#251); iOS no-op - PDFView.spacing for inter-page gaps when autoSpacing is true (#335), preserving platform defaults and top-alignment behavior Bump flutter_pdfview_platform_interface to 1.0.1 and flutter_pdfview to 1.5.0-beta.12.
endigo
changed the title
|
@macroscope-app review |
|
Manual reviews triggered for commit All prior checks · these links stay valid even if you push more commits. |
|
Review started. Results will be posted as check runs when complete. |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| return | ||
| } | ||
|
|
||
| pdfView.document = document |
There was a problem hiding this comment.
🟠 High flutter_pdfview/FlutterPDFView.swift:1163
reload() can leave the PDF permanently blank: when fitScaleForCurrentPolicy() returns 0 before PDFKit finishes layout, the method hides pdfView but never schedules a layout pass or restores visibility, yet still returns true. The rest of the codebase handles this helper returning 0 by deferring the fit to the next layoutSubviews, but this method does neither. Consider skipping the manual scale/visibility block when fitScale is not usable and letting setNeedsLayout()/layoutIfNeeded() drive the re-fit and reveal.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift around line 1163:
`reload()` can leave the PDF permanently blank: when `fitScaleForCurrentPolicy()` returns 0 before PDFKit finishes layout, the method hides `pdfView` but never schedules a layout pass or restores visibility, yet still returns `true`. The rest of the codebase handles this helper returning 0 by deferring the fit to the next `layoutSubviews`, but this method does neither. Consider skipping the manual scale/visibility block when `fitScale` is not usable and letting `setNeedsLayout()`/`layoutIfNeeded()` drive the re-fit and reveal.
| preventLinkNavigation = settings.bool("preventLinkNavigation") | ||
| } | ||
|
|
||
| if settings.keys.contains("enableSwipe") { |
There was a problem hiding this comment.
🟠 High flutter_pdfview/FlutterPDFView.swift:1280
Runtime updates to enableSwipe only toggle the scroll view's isScrollEnabled and never update pdfView.displayMode or usePageViewController. A view created with enableSwipe: false stays in .singlePage mode, so rebuilding with enableSwipe: true still cannot swipe between pages. Consider re-deriving useHorizontalPaging/displayMode and reapplying them when enableSwipe changes, or document that enableSwipe is only honored at creation time.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift around line 1280:
Runtime updates to `enableSwipe` only toggle the scroll view's `isScrollEnabled` and never update `pdfView.displayMode` or `usePageViewController`. A view created with `enableSwipe: false` stays in `.singlePage` mode, so rebuilding with `enableSwipe: true` still cannot swipe between pages. Consider re-deriving `useHorizontalPaging`/`displayMode` and reapplying them when `enableSwipe` changes, or document that `enableSwipe` is only honored at creation time.
| methodChannel.invokeMethod("onRender", args) | ||
| }.onDraw { _, _, _, _ -> | ||
| if (disposed) return@onDraw | ||
| val now = System.currentTimeMillis() |
There was a problem hiding this comment.
🟡 Medium pdfviewflutter/FlutterPDFView.kt:362
The onDraw throttle uses System.currentTimeMillis(), which is wall-clock time. When the system clock is corrected backward (by the user or network NTP), now - lastDrawTime becomes negative for the entire rollback duration, so the < DRAW_THROTTLE_MS guard suppresses all onDraw position/scale callbacks until the clock catches back up — potentially minutes or hours. Use a monotonic source such as SystemClock.uptimeMillis() or elapsedRealtime() for elapsed-time throttling.
- val now = System.currentTimeMillis()
+ val now = android.os.SystemClock.uptimeMillis()🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt around line 362:
The `onDraw` throttle uses `System.currentTimeMillis()`, which is wall-clock time. When the system clock is corrected backward (by the user or network NTP), `now - lastDrawTime` becomes negative for the entire rollback duration, so the `< DRAW_THROTTLE_MS` guard suppresses all `onDraw` position/scale callbacks until the clock catches back up — potentially minutes or hours. Use a monotonic source such as `SystemClock.uptimeMillis()` or `elapsedRealtime()` for elapsed-time throttling.
| } | ||
|
|
||
| /// True when the image is effectively solid white/empty (failed snapshot). | ||
| private func isMostlyBlank(_ image: UIImage) -> Bool { |
There was a problem hiding this comment.
🟠 High flutter_pdfview/FlutterPDFView.swift:1621
isMostlyBlank treats any non-white pixel as content, so a failed platform-layer snapshot is only detected as blank when the configured backgroundColor is near-white. When a dark or otherwise non-white backgroundColor is set and the layer render produces no PDF content, every sampled pixel satisfies r < 250 || g < 250 || b < 250, so the method declares the solid-background capture non-blank and capturePDFImage returns a solid-color image instead of using the page fallback. getScreenshot therefore produces a blank screenshot for hybrid-composition failures whenever a non-white background is configured.
The blankness check compares against white instead of the actual pdfView.backgroundColor. Consider comparing sampled pixels against the configured background color rather than fixed white thresholds.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift around line 1621:
`isMostlyBlank` treats any non-white pixel as content, so a failed platform-layer snapshot is only detected as blank when the configured `backgroundColor` is near-white. When a dark or otherwise non-white `backgroundColor` is set and the layer render produces no PDF content, every sampled pixel satisfies `r < 250 || g < 250 || b < 250`, so the method declares the solid-background capture non-blank and `capturePDFImage` returns a solid-color image instead of using the page fallback. `getScreenshot` therefore produces a blank screenshot for hybrid-composition failures whenever a non-white background is configured.
The blankness check compares against white instead of the actual `pdfView.backgroundColor`. Consider comparing sampled pixels against the configured background color rather than fixed white thresholds.
| } finally { | ||
| view.setLayerType(previousLayerType, null) | ||
| } |
There was a problem hiding this comment.
🟠 High pdfviewflutter/FlutterPDFView.kt:672
loadBitmapFromPDFView resets the view to light mode after a software-capture screenshot. It temporarily switches to LAYER_TYPE_SOFTWARE with null paint, and the finally block restores only the previous layer type, still passing null as the paint. In dark mode, applyColorTheme installed the ColorMatrixColorFilter via the hardware layer's Paint; restoring the layer with null paint discards that filter, so the live PDF reverts to light rendering until the theme is reapplied. This affects every screenshot that falls back to the software path (pre-API-26 devices and failed/unavailable PixelCopy). Preserve the theme paint when restoring the layer, or call applyColorTheme(false) in the finally block.
} finally {
- view.setLayerType(previousLayerType, null)
+ if (colorMatrix != null) {
+ val paint = Paint().apply {
+ colorFilter = ColorMatrixColorFilter(ColorMatrix(colorMatrix!!.copyOf()))
+ }
+ view.setLayerType(previousLayerType, paint)
+ } else {
+ view.setLayerType(previousLayerType, null)
+ }
}🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt around lines 672-674:
`loadBitmapFromPDFView` resets the view to light mode after a software-capture screenshot. It temporarily switches to `LAYER_TYPE_SOFTWARE` with `null` paint, and the `finally` block restores only the previous layer *type*, still passing `null` as the paint. In dark mode, `applyColorTheme` installed the `ColorMatrixColorFilter` via the hardware layer's `Paint`; restoring the layer with `null` paint discards that filter, so the live PDF reverts to light rendering until the theme is reapplied. This affects every screenshot that falls back to the software path (pre-API-26 devices and failed/unavailable `PixelCopy`). Preserve the theme paint when restoring the layer, or call `applyColorTheme(false)` in the `finally` block.
| // content offset / insets. Force free space below the page. | ||
| let contentHeight = scrollView.contentSize.height | ||
| let viewHeight = scrollView.bounds.height | ||
| if contentHeight > 0, contentHeight < viewHeight - 0.5 { |
There was a problem hiding this comment.
🟡 Medium flutter_pdfview/FlutterPDFView.swift:1001
pinContentToTop() sets contentInset.bottom via max(inset.bottom, extra) and never reduces it, so after a viewport shrink or orientation change the stale larger inset remains and creates a persistent scrollable blank region below the PDF. The extra inset added when content was shorter than the viewport is never recomputed or removed on subsequent calls. Consider resetting inset.bottom to the base value before re-applying the top-alignment contribution in both the short-content and tall-document branches.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift around line 1001:
`pinContentToTop()` sets `contentInset.bottom` via `max(inset.bottom, extra)` and never reduces it, so after a viewport shrink or orientation change the stale larger inset remains and creates a persistent scrollable blank region below the PDF. The extra inset added when content was shorter than the viewport is never recomputed or removed on subsequent calls. Consider resetting `inset.bottom` to the base value before re-applying the top-alignment contribution in both the short-content and tall-document branches.
| result.success(true) | ||
| } | ||
|
|
||
| fun setScale(call: MethodCall, result: Result) { |
There was a problem hiding this comment.
🟠 High pdfviewflutter/FlutterPDFView.kt:481
setScale skips view.zoomTo(...) when the requested scale is exactly 1.0, yet still returns success. After zooming to any other value, calling setScale with 1.0 cannot reset the document to its default scale, and a subsequent getScale() continues to return the previous zoom. The if (zoom != 1.0) guard prevents the zoom-to-1 case from being applied; remove the guard so view.zoomTo(zoom.toFloat()) runs for all values.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/android/src/main/kotlin/io/endigo/plugins/pdfviewflutter/FlutterPDFView.kt around line 481:
`setScale` skips `view.zoomTo(...)` when the requested scale is exactly `1.0`, yet still returns success. After zooming to any other value, calling `setScale` with `1.0` cannot reset the document to its default scale, and a subsequent `getScale()` continues to return the previous zoom. The `if (zoom != 1.0)` guard prevents the zoom-to-1 case from being applied; remove the guard so `view.zoomTo(zoom.toFloat())` runs for all values.
| `1.5.0-beta.10` continues the Kotlin/Swift line by letting password-protected | ||
| documents be unlocked from the app ([#274](https://github.com/endigo/flutter_pdfview/issues/274)), | ||
| on top of the luminance-preserving `PdfColorMode` dark theming in `beta.9` | ||
| ([#215](https://github.com/endigo/flutter_pdfview/issues/215), | ||
| [#138](https://github.com/endigo/flutter_pdfview/issues/138)). Pre-releases are not | ||
| picked up by a `^` constraint, so pin it explicitly: | ||
|
|
||
| ``` | ||
| dependencies: | ||
| flutter_pdfview: 1.5.0-beta.10 | ||
| ``` |
There was a problem hiding this comment.
🟢 Low flutter_pdfview/README.md:23
The beta installation instructions pin flutter_pdfview: 1.5.0-beta.10, but the current package version is 1.5.0-beta.12. Users who copy the README snippet install the older release and miss the beta.11/beta.12 changes (federated interface, animated setPage, spacing). The heading and pin should be updated to 1.5.0-beta.12.
-`1.5.0-beta.10` continues the Kotlin/Swift line by letting password-protected
+`1.5.0-beta.12` continues the Kotlin/Swift line by letting password-protected🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/README.md around lines 23-33:
The beta installation instructions pin `flutter_pdfview: 1.5.0-beta.10`, but the current package version is `1.5.0-beta.12`. Users who copy the README snippet install the older release and miss the beta.11/beta.12 changes (federated interface, animated `setPage`, `spacing`). The heading and pin should be updated to `1.5.0-beta.12`.
| }() | ||
|
|
||
| override func draw(with box: PDFDisplayBox, to context: CGContext) { | ||
| guard let owner = document?.delegate as? FlutterPDFView, owner.isDarkMode else { |
There was a problem hiding this comment.
🟡 Medium flutter_pdfview/FPVThemedPage.swift:86
draw(with:to:) reads owner.isDarkMode from PDFKit's background rendering thread while setColorMode writes the same property on the main thread, with no synchronization. A color-mode update concurrent with page rendering produces a data race, which is undefined behavior in Swift and can let a render observe a partially written value. Consider making isDarkMode thread-safe (e.g., a DispatchQueue barrier, atomic, or OSAllocatedUnfairLock) or snapshotting it under a lock before reading off the main thread.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FPVThemedPage.swift around line 86:
`draw(with:to:)` reads `owner.isDarkMode` from PDFKit's background rendering thread while `setColorMode` writes the same property on the main thread, with no synchronization. A color-mode update concurrent with page rendering produces a data race, which is undefined behavior in Swift and can let a render observe a partially written value. Consider making `isDarkMode` thread-safe (e.g., a `DispatchQueue` barrier, atomic, or `OSAllocatedUnfairLock`) or snapshotting it under a lock before reading off the main thread.
| // 2) Fallback: draw the current page into the viewport via PDFKit. | ||
| // This always has real page content even when the platform-view layer | ||
| // is not snapshot-friendly. | ||
| return renderer.image { ctx in |
There was a problem hiding this comment.
🟠 High flutter_pdfview/FlutterPDFView.swift:1593
capturePDFImage's fallback returns a screenshot of the whole current page fitted and centered in the viewport, ignoring pdfView.scaleFactor and the current scroll offset. When the preferred layer snapshot is classified as blank (the hybrid-composition case this fallback exists for), a zoomed-in or scrolled view produces a screenshot of the entire page rather than what the user is actually seeing. The fallback should apply the current pdfView.scaleFactor and scroll offset so the captured region matches the visible viewport.
🚀 Reply "fix it for me" or copy this AI Prompt for your agent:
In file @packages/flutter_pdfview/ios/flutter_pdfview/Sources/flutter_pdfview/FlutterPDFView.swift around line 1593:
`capturePDFImage`'s fallback returns a screenshot of the whole current page fitted and centered in the viewport, ignoring `pdfView.scaleFactor` and the current scroll offset. When the preferred layer snapshot is classified as blank (the hybrid-composition case this fallback exists for), a zoomed-in or scrolled view produces a screenshot of the entire page rather than what the user is actually seeing. The fallback should apply the current `pdfView.scaleFactor` and scroll offset so the captured region matches the visible viewport.
ApprovabilityVerdict: Needs human review 9 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
…p-align inset recompute M2: a runtime `enableSwipe` update only toggled `isScrollEnabled`, so a view created with `enableSwipe: false` stayed in `.singlePage` and could never swipe between pages. The paging derivation moves into `derivedPagingConfiguration()`, used by both creation and the update path; the runtime path re-applies `usePageViewController` / `displayMode`, re-adopts the scroll view PDFKit swaps in, and restores page and zoom. M4: blank detection compared samples against fixed near-white thresholds while the capture pre-fills with `pdfView.backgroundColor`, so a failed layer snapshot over a dark or tinted background read as ink and a solid-color image was returned instead of the page fallback. Samples are now measured against the background actually filled, with the same tolerance the white threshold had. M6: `pinContentToTop` only ever grew the bottom inset, leaving a stale blank scroll region after an orientation change or a zoom past the viewport. The inset is recomputed from the base inset plus the current extra, and the tall-document branch gives back the contribution — the iPad's automatic adjustment lands in `adjustedContentInset` and is untouched. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GtmxBdKLoSEzKzS6LhYQ1A
1 participant
Summary
This PR is the full 1.5.0 beta line on top of stable 1.4.5: native language migration, the major feature/fix stack, the federated Melos monorepo, and late community PR rewrites.
Published on pub.dev
flutter_pdfview_platform_interface1.0.1 (must publish first; 1.0.0 also live)flutter_pdfview1.5.0-beta.12App install (pre-release must be pinned explicitly):
Public import path is unchanged:
package:flutter_pdfview/flutter_pdfview.dart. Value types are re-exported from the platform interface.1. Native migration (beta.1 + follow-ups)
Android: Java → Kotlin
packages/flutter_pdfview/android/.../kotlin/…(FlutterPDFView, factory, plugin, link handler,PdfColorMatrix)iOS: Objective-C → Swift 5.9
FlutterPDFView.swift,PDFViewFlutterPlugin.swift,FPVThemedPage.swiftFLTPDFViewFlutterPluginkept via@objcFPVExceptionCatcherObj-C shim for PDFKitNSException(SPM separate target; CocoaPods mixed sources)file:URI handling, fit/autoSpacing(Not able to show well fitted PDF at iOS, but OK for Android #150), hardenedsetZoomLimits, block-based scroll KVO withdeinitinvalidation2. Features (beta.2–beta.12)
PdfColorMode(light/dark/system)nightModedeprecated; runtime color/background updates without remountonPasswordRequired+PDFViewController.unlock(); retry without remountingPageAlignment(center/top)setPagere-centers secondary axisonTapsetPage(withAnimation:)jumpTo; iOS ignores (PDFKit no-op). DefaultfalsespacingautoSpacingis true;nullkeeps historical defaults + top-align gaps3. Bugfixes & quality
getScreenshotwhite under hybrid compositionthumbnailRatio0.8, AA)Docs only: iOS platform-view filter limits (#213 · #352); Android AcroForm
/APlimits (#303 · #354).Superseded / already fixed (closed without merge):
minZoom/maxZoom/setZoomLimitssince 1.4.54. Federated monorepo (beta.11 · #360)
packages/flutter_pdfview/flutter_pdfview_platform_interface(^1.0.1):FlutterPdfViewPlatform,PdfViewPlatformControllerPdfViewSettings/ creation params / callbacks (wire format shared)MethodChannelFlutterPdfViewFitPolicy,PageAlignment,PdfColorMode,PDFPasswordFailureFlutterFrameworkinPackage.swift(Flutter 3.44+)kotlin-androidonly on AGP 8;kotlin { compilerOptions { jvmTarget } }for AGP 8/9Version map on this branch
onTap(#133)PageAlignment+ setPage center (#250 #272 #197)PdfColorModestack (#215 #138)setPage(withAnimation:)(#251) +spacing(#335); interface 1.0.1(beta.8–.9 were folded into later published tags; pub.dev has beta.1–.7, .10–.12.)
Test plan
flutter analyzeclean on workspace packagespackages/flutter_pdfview+ platform interface)dart pub publish --dry-run(0 warnings) for both packagessetPage()method for Android #251/feat:Add spacing parameter to PDF viewer for customizable page spacing #335 rewritten and closedRelated
setPage()method for Android #251, feat:Add spacing parameter to PDF viewer for customizable page spacing #335 (closed as completed by rewrite); feat: add maxZoom support for iOS and Android #326, fix(ios): avoid double-initializing FLTPDFView #332 (closed as already done)Note
Migrate flutter_pdfview to a federated monorepo with Kotlin/Swift native implementations and new 1.5.0 features
packages/flutter_pdfviewandpackages/flutter_pdfview_platform_interface, managed by melos.flutter_pdfview_objc) for exception bridging.FlutterPdfViewPlatform,PdfViewPlatformController,PdfViewSettings,PdfViewCallbacks,PdfViewCreationParams) that decouples the Dart API from method-channel details.PDFViewwidget properties:colorMode(PdfColorMode.light/dark/system, replacing deprecatednightMode),pageAlignment,spacing,onPasswordRequired, andonTap; color mode auto-updates when the system theme changes viadidChangeDependencies.onPasswordRequireddelivers aPDFPasswordFailurereason andPDFViewController.unlock(password)sends the credential to native.nightModeis deprecated andcolorMode: PdfColorMode.systemis the new default; existing code usingnightMode: truestill works but should migrate tocolorMode.Macroscope summarized e581ee4.