Skip to content

[flutter_inappwebview] Fix navigation/getUrl/scroll bugs and add integration tests - #1083

Open
seungsoo47 wants to merge 8 commits into
flutter-tizen:masterfrom
seungsoo47:flutter_inappwebview-nav-getUrl-fix
Open

[flutter_inappwebview] Fix navigation/getUrl/scroll bugs and add integration tests#1083
seungsoo47 wants to merge 8 commits into
flutter-tizen:masterfrom
seungsoo47:flutter_inappwebview-nav-getUrl-fix

Conversation

@seungsoo47

Copy link
Copy Markdown
Contributor
  • Fix onTitleChanged to fire on later title changes (e.g. via JS document.title), not just once after load.
  • Fix a getUrl() race with cancelled navigations, and skip the shouldOverrideUrlLoading round-trip for app-initiated navigations (loadUrl, goBack, reload, etc.).
  • Fix scrollBy/getScrollX/getScrollY occasionally returning a stale position right after scrollTo/scrollBy (EWK applies scroll asynchronously).
  • Add Tizen integration tests ported from upstream flutter_inappwebview v6.1.5.
  • Bump flutter_inappwebview_tizen to 0.2.0.

…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.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 6bb2e06498

ℹ️ 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".

Comment thread packages/flutter_inappwebview/tizen/src/webview.cc
Comment thread packages/flutter_inappwebview/tizen/src/webview.cc Outdated
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.

Found by chatgpt-codex-connector's review on PR flutter-tizen#1083.
@seungsoo47

Copy link
Copy Markdown
Contributor Author

@codex review

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e7dcbb1442

ℹ️ 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".

Comment thread packages/flutter_inappwebview/tizen/src/webview.cc Outdated
Comment thread packages/flutter_inappwebview/tizen/src/webview.cc
…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.

Found by chatgpt-codex-connector's review on PR flutter-tizen#1083.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant