[flutter_inappwebview] Fix navigation/getUrl/scroll bugs and add integration tests - #1099
Merged
seungsoo47 merged 10 commits intoAug 25, 2026
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0f0bf8f33c
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
JSUYA
reviewed
Aug 21, 2026
…age load The Tizen implementation only reported the page title once, right after a page finished loading. It never listened for the WebView's own title-changed notifications, so title updates made afterwards (for example by JavaScript setting document.title) were never reported to onTitleChanged. Register a "title,changed" listener on the underlying webview instance, matching the pattern already used for load and navigation events, so onTitleChanged fires whenever the title actually changes.
…ound-trip on programmatic navigation OnNavigationPolicy always suspended the view and asked Dart's shouldOverrideUrlLoading whether to allow a navigation, even for navigations the app itself requested (loadUrl, goBack, reload, ...). That round-trip is meant for user/page-initiated navigation only. Also, when Dart calls stopLoading() to cancel a pending navigation, getUrl() had no way to know a cancellation happened: EWK's "url,changed" event can still fire for the cancelled URL (before or after ewk_view_stop() takes effect), so getUrl() could end up reporting a URL the app never actually finished navigating to. Fix both: - Every EWK call that starts an app-requested navigation now goes through NavigateProgrammatically(), which marks the navigation as programmatic. OnNavigationPolicy checks this flag and accepts immediately, skipping the shouldOverrideUrlLoading round-trip for it. - StopNavigation() records that the current navigation was cancelled and reverts committed_url_ to the URL snapshotted just before the navigation decision was accepted (pending_navigation_revert_url_). OnUrlChange ignores "url,changed" while a cancellation is pending, and getUrl() returns committed_url_ instead of asking EWK directly in that window.
…tale position ewk_view_scroll_pos_get() right after ewk_view_scroll_set() can return the pre-scroll position because EWK applies the scroll asynchronously, so scrollBy's delta and getScrollX/getScrollY's return value were sometimes stale by one frame. Track the last requested scroll position in target_scroll_x_/y_ and use it as the source of truth until EWK's reported position catches up with it, then fall back to querying EWK directly. Reset both to -1 on navigation start/error since a new page invalidates any pending scroll target.
Add Tizen-compatible test cases derived from the upstream
flutter_inappwebview v6.1.5 integration test suite, covering the parts
of the API that the Tizen implementation actually supports (the
InAppWebView widget/controller and CookieManager.deleteAllCookies).
Most of upstream's suite exercises features this plugin does not
implement (in-app browser, Chrome Custom Tabs, headless webview, find
interaction, service worker, proxy, tracing, process-global config,
the local asset-loader server, and most Android/iOS-only settings and
callbacks), so those tests don't apply here and were left out.
New test cases, alongside the 4 already in the file:
- getProgress reports 100 once the page finishes loading
- reload reloads the currently displayed page
- loadUrl navigates to a new URL
- postUrl and loadUrl submit an HTTP POST request body
- loadFile loads a bundled asset file
- programmatic scroll updates and reports the scroll position
- onScrollChanged fires when the scroll position changes
- onTitleChanged fires when document.title changes
- stopLoading interrupts an in-flight page load
- clearAllCache completes without throwing
- zoomBy triggers onZoomScaleChanged
- onReceivedError reports a host lookup failure / is not raised for a
successful load
- setSettings applies updated webview settings
The new tests reuse the file's existing local HTTP server fixture
instead of upstream's live external URLs, so they stay reliable on a
TV emulator or device without depending on outside network resources.
A small bundled HTML asset was added for the loadFile case.
Making the onTitleChanged test pass required fixing a gap in the
plugin itself (separate commit): it only reported the title once,
right after a page finished loading, and never listened for later
title changes such as JavaScript setting document.title.
Validated with `flutter-tizen drive` on a Raspberry Pi device (all 17
cases pass). flutter_inappwebview is currently marked disabled for
the TV emulator profile in .github/recipe.yaml because of a separate,
unrelated crash on WebView disposal there; that is out of scope for
this change.
The postUrl/loadUrl body assertions poll for the expected text via
_waitForCondition instead of reading document.querySelector('p')
immediately, since the page's DOM update after a POST/navigation
isn't synchronous with the awaited call and the immediate read was
occasionally flaky.
goBack/goForward always reported success to NavigateProgrammatically regardless of whether ewk_view_back()/ewk_view_forward() actually had history to navigate. When called with no history, no navigation policy callback ever fires to clear is_programmatic_navigation_, so the flag leaks into the next user-initiated navigation and incorrectly skips shouldOverrideUrlLoading. Use the EWK calls' own return value instead. getScrollX/getScrollY kept substituting the requested scrollTo/scrollBy target for the actual position until they matched, to mask EWK applying scroll asynchronously. If the requested position is beyond the page's max scroll extent, EWK clamps it and the actual position never matches the target, so out-of-range coordinates were reported indefinitely. Mask only the single read immediately following a scroll instead.
…getUrl after loadData The XF86Back (remote/hardware Back key) handler wrapped ewk_view_back() in NavigateProgrammatically, marking it as a programmatic navigation. OnNavigationPolicy takes the early-accept path for programmatic navigations and never calls shouldOverrideUrlLoading, so apps could not intercept or block a user-initiated Back-key navigation even with useShouldOverrideUrlLoading enabled. Call ewk_view_back() directly so it goes through the normal navigation-policy path, matching goBack() being the only case that should bypass the delegate. is_navigation_cancelled_ (set by StopNavigation() when a delegate cancels a navigation) is only cleared by OnNavigationPolicy. loadData() calls ewk_view_html_string_load(), which never triggers OnNavigationPolicy, so calling loadData() after a cancelled navigation left the flag stuck and getUrl() kept returning the pre-cancellation URL even though new content had loaded. Clear the flag before the html_string_load call.
- Fix getScrollX/getScrollY clearing both axis targets on a single call, which reintroduced the stale-scroll-position bug this PR fixes. - Remove the redundant onTitleChanged emission in OnLoadFinished now that the title,changed callback reports it, avoiding a duplicate event on a normal load. - Skip snapshotting the pre-navigation URL when there's no navigation delegate, since the shouldOverrideUrlLoading race it guards against doesn't apply there. - Rename pending_navigation_revert_url_ to url_before_navigation_ and clarify comments around the suspend/resume and programmatic-nav skip in OnNavigationPolicy/StopNavigation. - Tighten CHANGELOG entries to single-line bullets.
…le scroll targets is_navigation_cancelled_ stayed set until the next navigation-policy decision or loadData() call, so OnUrlChange() silently dropped every subsequent "url,changed" event, not just the one stale event caused by the cancellation itself. A same-document change (history.pushState/ replaceState) on the still-loaded page was permanently ignored until a full navigation occurred. Clear the flag after suppressing the first stale event. target_scroll_x_/y_ were only invalidated by a getter call or a page load, so a scrollBy() issued without an intervening getter could compose off a target left over from an earlier scrollTo() even after the user scrolled manually or EWK clamped an out-of-range value. Bound how long a pending target is trusted so a stale one falls back to the real position EWK reports.
seungsoo47
force-pushed
the
flutter_inappwebview-nav-getUrl-fix2
branch
from
August 25, 2026 09:13
0f0bf8f to
756b622
Compare
JSUYA
approved these changes
Aug 25, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
onTitleChangedto fire on later title changes (e.g. via JS document.title), not just once after load.