Skip to content

Comments

feat(android): trigger Event::Opened#1155

Draft
lucasfernog wants to merge 4 commits intofeat/mobile-multi-windowfrom
feat/opened-event-android
Draft

feat(android): trigger Event::Opened#1155
lucasfernog wants to merge 4 commits intofeat/mobile-multi-windowfrom
feat/opened-event-android

Conversation

@lucasfernog
Copy link
Member

No description provided.

@lucasfernog lucasfernog requested a review from a team as a code owner November 17, 2025 15:16
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2025

Package Changes Through 2268fac

There are 1 changes which include tao with minor

Planned Package Versions

The following package releases are the planned based on the context of changes in this pull request.

package current next
tao 0.34.5 0.35.0

Add another change file through the GitHub UI by following this link.


Read about change files or the docs at github.com/jbolda/covector

lucasfernog added a commit to tauri-apps/wry that referenced this pull request Nov 17, 2025
@lucasfernog
Copy link
Member Author

needs #1154 to be merged first

}

#[allow(non_snake_case)]
pub unsafe fn onNewIntent(env: JNIEnv, _: JClass, intent: JObject) {

Choose a reason for hiding this comment

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

There are a number of silent failure paths in this function e.g.

let Ok(action) = env.get_string(&action)
  .map(|action| action.to_string_lossy().to_string())
else {
  return; // Silent failure
};

Should these errors be propagated or logged?

Copy link
Member Author

Choose a reason for hiding this comment

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

pub(crate) static ref CONTEXTS: Mutex<BTreeMap<ActivityId, AndroidContext>> = Mutex::new(Default::default());
static ref WINDOW_MANAGER: Mutex<BTreeMap<ActivityId, GlobalRef>> = Mutex::new(Default::default());
pub(crate) static ref ACTIVITY_CREATED_SENDERS: Mutex<BTreeMap<ActivityId, Sender<()>>> = Mutex::new(Default::default());
static ref INTENT_URLS: Mutex<Vec<url::Url>> = Mutex::new(Default::default());

Choose a reason for hiding this comment

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

Using global mutable state for intent URLs. How does this interact with multi-window support? What if multiple windows exist? How does the intent get delivered to the correct window?

Copy link
Member Author

Choose a reason for hiding this comment

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

the intent URL is not delivered to a particular window - it is instead sent as a global event Event::Opened
i wanted to include the URLs in the android Event enum but I can't since it has to implement Copy - that's why I'm using this global variable.


/// Character set for encoding text content in data URLs.
/// Encodes all control characters and special characters that might cause issues in URLs.
const DATA_URL_ENCODING_SET: &AsciiSet = &CONTROLS

Choose a reason for hiding this comment

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

Tauri converts shared text to data URLs to provide a unified Event::Opened { urls: Vec<Url> } API across all content types (deep links, files, and text). Is there a specific reason why this pattern is used?

Other cross-platforms frameworks (such as React Native) handle this by splitting out the different content types e.g.

pub enum Event {
    Opened { urls: Vec<Url> },        // Deep links & file URIs only
    TextShared { text: String },       // Raw shared text
    // ...
}

Alternatively a tagged union could be used to handle the different content types:

pub enum OpenedContent {
    Url(Url),
    Text(String),
}

pub enum Event {
    Opened { content: Vec<OpenedContent> },
    // ...
}

Copy link
Member Author

Choose a reason for hiding this comment

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

Is there a specific reason why this pattern is used?

backwards compatibility

tho the new variant is probably ok, would just be a little weird to have two different paths for the same interface (intent data)

let mut urls = HashSet::new();

// Get intent type (may be null)
let intent_type: JString = env

Choose a reason for hiding this comment

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

This defaults to an empty string if null. Would it be better to use a Option<string> here?

Copy link
Member Author

Choose a reason for hiding this comment

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

doesn't matter, the type is not exposed, we just check it to see the data format, but i'll change it anyway

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.

2 participants