-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[url_launcher][web] Prevent browser from navigating when followLink isn't called #8675
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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:
|
…llowLink isn't called (flutter/packages#8675)
flutter/packages@c44c228...01d3d5c 2025-02-27 engine-flutter-autoroll@skia.org Manual roll Flutter from 043b719 to 1659206 (19 revisions) (flutter/packages#8728) 2025-02-27 stuartmorgan@google.com Manual roll Flutter from 911aa75 to 043b719 (flutter/packages#8693) 2025-02-26 magder@google.com Dependabot to update major and minor versions of test dependencies, ignore patch (flutter/packages#8712) 2025-02-26 reidbaker@google.com [local_auth] Update to use flutter.targetSdkVersion (flutter/packages#8695) 2025-02-26 57854043+CaoGiaHieu-dev@users.noreply.github.com [go_router_builder]: Handle invaild params (flutter/packages#8405) 2025-02-26 stuartmorgan@google.com [pigeon] Timestamp test steps in CI (flutter/packages#8716) 2025-02-26 43054281+camsim99@users.noreply.github.com [camera_android_camerax] Fix 90°-off preview rotation (flutter/packages#8629) 2025-02-26 108667553+aprzedecki@users.noreply.github.com [go_router] Secured empty matches in canPop (flutter/packages#8557) 2025-02-26 reidbaker@google.com [tool] Update targetsdk version to 35 from 32 (flutter/packages#8694) 2025-02-26 stuartmorgan@google.com [various] Bump androidx.test:core to 1.4.0 (flutter/packages#8710) 2025-02-26 stuartmorgan@google.com [camera] Disable flaky tests (flutter/packages#8708) 2025-02-26 mdebbar@google.com [url_launcher][web] Prevent browser from navigating when followLink isn't called (flutter/packages#8675) 2025-02-26 stuartmorgan@google.com [various] Remove plugin-level `integration_test` dependencies (flutter/packages#8711) 2025-02-26 stuartmorgan@google.com [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 flutter-ecosystem@google.com 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
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:[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.[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 thefollowLink
signal. If the signal arrives a bit later, we fallback to a programmatic navigation through thelaunchUrl
API.Fixes flutter/flutter#162927
Fixes flutter/flutter#162408
Pre-launch Checklist
dart format
.)[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.md
to 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.