Skip to content
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

Add onDeepLink EventHandler to h5vcc_runtime IDL #4993

Merged

Conversation

zhongqiliang
Copy link
Contributor

@zhongqiliang zhongqiliang commented Feb 28, 2025

  1. Modify h_5_vcc_runtime.idl to include an onDeepLink event handler.
  2. Define a custom event named DeepLinkEvent to be used by the onDeepLink handler. This event will carry the deep link information.
  3. When a JavaScript application registers the onDeepLink event handler. Attempt to retrieve the initial deep link using the GetInitialDeepLink() mojom API. If the retrieved value is not empty, immediately dispatch a DeepLinkEvent with the initial deep link data."

Sample javascript code for register the onDeepLink callback.

window.h5vcc.runtime.onDeepLink = (event) => {
  if (event.type === 'deeplink') {
    console.log("Deeplink event received!");
    console.log("Deeplink URL:", event.url);
  }
};

Tested on devtools. https://screenshot.googleplex.com/AymzZtjCxUYZg2G

b/374147993

@zhongqiliang zhongqiliang requested a review from a team as a code owner February 28, 2025 07:03
@zhongqiliang
Copy link
Contributor Author

Working on the web test.

@zhongqiliang zhongqiliang requested review from yell0wd0g, kaidokert, hlwarriner and andrewsavage1 and removed request for dahlstrom-g February 28, 2025 07:05
@zhongqiliang zhongqiliang force-pushed the add_ondeeplink_eventhandler_2 branch from c4bea6d to 079eee4 Compare February 28, 2025 07:08
Copy link
Contributor

@yell0wd0g yell0wd0g left a comment

Choose a reason for hiding this comment

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

Looking pretty good! A few comments here and there.

Copy link
Contributor

@yell0wd0g yell0wd0g left a comment

Choose a reason for hiding this comment

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

I don't oppose these changes anymore but I'll ask for another pass at the JS test 🙏

Copy link
Contributor

@hlwarriner hlwarriner left a comment

Choose a reason for hiding this comment

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

Left two minor comments but otherwise lgtm, thanks!

@zhongqiliang zhongqiliang enabled auto-merge March 4, 2025 17:52
@andrewsavage1
Copy link
Contributor

Would this type of event handler change be what we follow for all of our event handlers? I.e. would the signature for H5vccRuntime.onPause end up changing a similar way?

@zhongqiliang zhongqiliang merged commit e11b7cb into youtube:main Mar 4, 2025
136 checks passed
@hlwarriner
Copy link
Contributor

@zhongqiliang I'm seeing a crash on a devel linux build at https://github.com/youtube/cobalt/blob/main/third_party/blink/renderer/core/trustedtypes/trusted_type_policy_factory.cc#L77, related to onDeepLink. Can you please take a look? We might need to revert or fix forward.

zhongqiliang added a commit that referenced this pull request Mar 4, 2025
#4993 can crash the app with this
error.

Abort message:
'[15412:15479:0304/132646.498142:FATAL:trusted_type_policy_factory.cc(77)]
Check failed: String(entry.attribute).LowerASCII() == entry.attribute
("ondeeplink" vs. onDeepLink)'

b/374147993
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants