Skip to content

[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

Merged
merged 1 commit into from
Feb 26, 2025

Conversation

mdebbar
Copy link
Contributor

@mdebbar mdebbar commented Feb 21, 2025

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

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

Copy link
Member

@ditman ditman left a 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 :)

Comment on lines +1051 to +1059
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);
Copy link
Member

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?

Copy link
Contributor Author

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();
});
Copy link
Member

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?

Copy link
Contributor Author

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.

@mdebbar
Copy link
Contributor Author

mdebbar commented Feb 25, 2025

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 :)

Agreed! To explain the reason behind the complication, there are 2:

  1. We want to comply with the framework's hit testing behavior. E.g. a circular button may create a rectangular <a> tag. So a simple click listener on the <a> tag will trigger the link even when the click didn't land on the actual circular button. This is why we need to wait for 2 signals: DOM event + followLink (the latter is called when the app/framework register a hit test).

  2. Semantics mode adds another layer of complexity. One example is the click debouncer affecting the order and timing of events.

@mdebbar mdebbar added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 25, 2025
@auto-submit auto-submit bot merged commit a2d2f16 into flutter:main Feb 26, 2025
82 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 27, 2025
github-merge-queue bot pushed a commit to flutter/flutter that referenced this pull request Feb 27, 2025
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
androidseb pushed a commit to androidseb/packages that referenced this pull request Jun 8, 2025
…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
FMorschel pushed a commit to FMorschel/packages that referenced this pull request Jun 9, 2025
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: url_launcher platform-web
Projects
None yet
2 participants