[url_launcher][web] Prevent browser from navigating when followLink isn't called #8675
Conversation
ditman
left a comment
ditman
left a comment
There was a problem hiding this comment.
Ship it! This code is getting super complicated to just follow a Link on the web, but I guess there are considerations from other platforms that I'm not taking into consideration :)
| final html.Element semanticsHost = | ||
| html.document.createElement('flt-semantics-host'); | ||
| html.document.body!.append(semanticsHost); | ||
| final html.Element semanticsAnchor = html.document.createElement('a') | ||
| ..setAttribute('id', 'flt-semantic-node-99') | ||
| ..setAttribute('flt-semantics-identifier', 'test-link-71') | ||
| ..setAttribute('href', uri.toString()) | ||
| ..textContent = 'My Text Link'; | ||
| semanticsHost.append(semanticsAnchor); |
There was a problem hiding this comment.
Is this needed because we can't turn on semantics in the test, so the engine creates all the semantics nodes for the Link?
There was a problem hiding this comment.
Semantics mode doesn't work end-to-end in tests, so I have to mimic what the engine would've created.
| } | ||
| canBrowserNavigate = false; | ||
| mouseEvent.preventDefault(); | ||
| }); |
There was a problem hiding this comment.
Instead of calling _scheduleReset(), should we just call reset directly at the beginning of this microtask?
In this case we really don't have to wait a "long" timeout to reset the state of the plugin, do we? Is it because of how followLink might happen at a random time?
There was a problem hiding this comment.
Yeah, we have to wait for followLink, which may (or may not) come later. So the timer is really acting like a deadline for waiting for followLink. If followLink doesn't show up by that time, we assume it's never coming.
In the normal case, followLink should arrive before the end of the microtask and everything works beautifully. But in semantics mode, we have a thing called "click debouncer" that delays pointer events. That's what causes the framework's onTap callback to be fired late.
Agreed! To explain the reason behind the complication, there are 2:
|
mdebbar
added
the
autosubmit
…owLink isn't called (flutter/packages#8675)
flutter/packages@c44c228...01d3d5c 2025-02-27 [email protected] Manual roll Flutter from 043b719 to 1659206 (19 revisions) (flutter/packages#8728) 2025-02-27 [email protected] Manual roll Flutter from 911aa75 to 043b719 (flutter/packages#8693) 2025-02-26 [email protected] Dependabot to update major and minor versions of test dependencies, ignore patch (flutter/packages#8712) 2025-02-26 [email protected] [local_auth] Update to use flutter.targetSdkVersion (flutter/packages#8695) 2025-02-26 [email protected] [go_router_builder]: Handle invaild params (flutter/packages#8405) 2025-02-26 [email protected] [pigeon] Timestamp test steps in CI (flutter/packages#8716) 2025-02-26 [email protected] [camera_android_camerax] Fix 90°-off preview rotation (flutter/packages#8629) 2025-02-26 [email protected] [go_router] Secured empty matches in canPop (flutter/packages#8557) 2025-02-26 [email protected] [tool] Update targetsdk version to 35 from 32 (flutter/packages#8694) 2025-02-26 [email protected] [various] Bump androidx.test:core to 1.4.0 (flutter/packages#8710) 2025-02-26 [email protected] [camera] Disable flaky tests (flutter/packages#8708) 2025-02-26 [email protected] [url_launcher][web] Prevent browser from navigating when followLink isn't called (flutter/packages#8675) 2025-02-26 [email protected] [various] Remove plugin-level `integration_test` dependencies (flutter/packages#8711) 2025-02-26 [email protected] [ci] Lengthen custom tests timeout (flutter/packages#8715) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…sn't called (flutter#8675) When a DOM click event is received, we need to make a decision before the end of the event loop whether to allow the browser to navigate or not. At the end of the event loop, the browser will perform the navigation if we don't prevent it. The problem occurs when the `followLink` signal arrives AFTER the event loop (this can happen in semantics mode when the web engine uses a debouncer to queue events and send them to the framework after some delay). This leads to a situation where we can't make a definitive decision by the end of the event loop, so this PR does the following: 1. [best case] If the `followLink` signal is received before the end of the event loop, we let the browser do the navigation for a full web link experience. 2. [meh case] If no `followLink` signal is received before the end of the event loop, we PREVENT the browser from navigating. But we keep waiting for the `followLink` signal. If the signal arrives a bit later, we fallback to a programmatic navigation through the `launchUrl` API. Fixes flutter/flutter#162927 Fixes flutter/flutter#162408
…sn't called (flutter#8675) When a DOM click event is received, we need to make a decision before the end of the event loop whether to allow the browser to navigate or not. At the end of the event loop, the browser will perform the navigation if we don't prevent it. The problem occurs when the `followLink` signal arrives AFTER the event loop (this can happen in semantics mode when the web engine uses a debouncer to queue events and send them to the framework after some delay). This leads to a situation where we can't make a definitive decision by the end of the event loop, so this PR does the following: 1. [best case] If the `followLink` signal is received before the end of the event loop, we let the browser do the navigation for a full web link experience. 2. [meh case] If no `followLink` signal is received before the end of the event loop, we PREVENT the browser from navigating. But we keep waiting for the `followLink` signal. If the signal arrives a bit later, we fallback to a programmatic navigation through the `launchUrl` API. Fixes flutter/flutter#162927 Fixes flutter/flutter#162408
2 participants
When a DOM click event is received, we need to make a decision before the end of the event loop whether to allow the browser to navigate or not. At the end of the event loop, the browser will perform the navigation if we don't prevent it.
The problem occurs when the
followLinksignal arrives AFTER the event loop (this can happen in semantics mode when the web engine uses a debouncer to queue events and send them to the framework after some delay). This leads to a situation where we can't make a definitive decision by the end of the event loop, so this PR does the following:[best case] If the
followLinksignal is received before the end of the event loop, we let the browser do the navigation for a full web link experience.[meh case] If no
followLinksignal is received before the end of the event loop, we PREVENT the browser from navigating. But we keep waiting for thefollowLinksignal. If the signal arrives a bit later, we fallback to a programmatic navigation through thelaunchUrlAPI.Fixes flutter/flutter#162927
Fixes flutter/flutter#162408
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.