ReviewOS

also looking at this

craft-native/craft

fix(android): answer the page from haptic, vibrate and the speech calls

#234
Merged glennmichael123 wants to merge fix/android-feedback-promises into main
7 files +364 -114
9 days ago

Closes #219.

What was wrong

haptic, vibrate, startListening and stopListening returned Unit from Kotlin and nothing from the page, so a page that awaited one got undefined — which reads as success — where iOS returns a promise since #207. The same code behaved differently on the two platforms.

startListening was worse than silent. Without RECORD_AUDIO it asked for the permission and then dropped the answer: no craftSpeechError, and no recogniser after the grant either. The page waited on an event that was never coming.

What it does now

A @JavascriptInterface method answers its caller synchronously, so there is no channel to park, unlike share and the rest. Each Kotlin method returns whether the device took the call, and the page hands that back:

haptic: function(style) {
    return Promise.resolve().then(function() { return CraftAndroid.haptic(style || 'medium') === true; });
},

Two details in that one line:

  • The call sits inside .then, not in Promise.resolve(...). index.test.ts has had a ratchet against the second form since the promise work: a native call that throws has to reject, not throw at the caller. Same shape as every other promise in this file.
  • === true, because a WebView reports some native failures to the page as undefined rather than as a throw, and undefined is exactly what this issue is about.

false means nothing played — haptics are off, or the vibrator refused. It is not an error: a haptic decorates a flow rather than carries it.

The #209 gate

haptic, vibrate and startListening now have a promise to reject, so they move from the gate's warn-and-return-nothing branch to CAPABILITY_DISABLED. #209 left that note in the comment; this collects it. What is left answering nothing is flashlight, keepAwake and the flat watch API (#220).

stopListening leaves the gate entirely, matching iOS. Stopping something an app was built without is a no-op, not a refusal — a page tearing a screen down should not have to know which build it is running on.

craft.haptics.*

Android could not draw #207's split until craft.haptic() had an answer to give. Now it can, and it draws the same one: the raw call reports the refusal, the feedback helpers treat a capability left off as nothing to play, and any other failure still rejects. Without this, every app that never set enableHaptics would have started failing on await haptics.selection() — the exact regression #207 found on iOS.

notification('warning') maps to medium here as it does on iOS, instead of falling in with light.

The microphone grant

iOS starts recording from inside its SFSpeechRecognizer.requestAuthorization callback. Android now does the same from onRequestPermissionsResult under a new REQUEST_SPEECH code, and sends craftSpeechError when the permission is denied. The recogniser itself moved into beginSpeechRecognition() so both paths reach it.

startListening answers true while it is asking, because the request was taken — the same thing iOS answers while its own authorization is in flight. The outcome arrives as craftSpeech* events either way.

stopListening sends craftSpeechEnd, and only when something was listening, so a page that stops twice does not see a session end twice.

Tests that fail without the fix

packages/android/src/page-script.test.ts runs the real injected script under Bun against a fake CraftAndroid that can now answer, or throw. Four mutations were run against it:

mutationfails
haptic returns nothing again5 tests
haptic back to un-awaited in the gate4
haptics.impact passes the refusal on2
stopListening gated again1

It also covers the parts only reachable off a device: that a native throw rejects rather than throwing at the call site, that it still reaches haptics.impact while a refusal does not, and that vibrate sends the pattern as the JSON Kotlin parses.

packages/android/src/index.test.ts pins the four Kotlin signatures, the grant continuation, and craftSpeechEnd.

The Android E2E suite gains the four cases iOS got in #207, and enableHaptics goes on in the probe config so haptic and vibrate have a success path to answer from. Speech stays off, so startListening has a refusal to carry:

caseasserts
haptic.resolvesTruetrue with haptics on
vibrate.resolvesTruetrue. Sends [100, 50, 100], not iOS's [], which VibrationEffect.createWaveform rejects
speech.disabled.rejectsCAPABILITY_DISABLED, the refusal the call could not carry before
speech.stopListening.resolvesTruetrue with speech off — the one speech success path an emulator reaches without a microphone

The Kotlin compile and the emulator legs run only in CI, as on #232 and #233.

Not done here

The permission prompt still only appears on the second startListening if the first was what triggered the request — the page is told, rather than kept waiting, which is what this issue asked for. Anything further belongs with #220's look at the watch API.

Verification

bun run test, bun run typecheck, bun run verify:packages, Pickier (0 errors), the android/ios/typescript package tests (98 and 598), and 55 protocol tests.

7 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.