Skip to content

feat(ohos): window ops bridge + status readback + FloatPage decorations - #45

Open
ddxwzc-boop wants to merge 6 commits into
Eulogizethesun:ohdevfrom
ddxwzc-boop:ohdev
Open

feat(ohos): window ops bridge + status readback + FloatPage decorations#45
ddxwzc-boop wants to merge 6 commits into
Eulogizethesun:ohdevfrom
ddxwzc-boop:ohdev

Conversation

@ddxwzc-boop

Copy link
Copy Markdown
  • window/mod.rs: bridge set_window_topmost/title/limits/ime_position/draggable/redraw/request_user_attention; instanceKey for specified launchType
  • app.rs: notify_window_status NAPI queue + drain (windowStatusChange backfill)
  • event.rs: FoldDisplayModeChange MainEvent
  • ArkHelper.ets: ArkTS impls for the new window ops
  • NativeAbility.ets + FloatPage.ets: windowStatusChange registration + seed; FloatPage title/min/max/close buttons, startMoving onTouch
  • WindowManager.ets: showMainAbility(startAbility instanceKey=main), setPointerStyle real windowId, decoration flag interception (closable/maximizable/minimizable/resizable), setWindowTitle/Limits/Topmost
  • type.ets: ArkHelper interface + UIAbilityWantParams.instanceKey

另含 fix(arkhelper): NAPI-reentrant sync 路径改用 safeLogError (C5)

The four newly-added bridge methods (requestRedraw, requestUserAttention,
setImePosition, setWindowDraggable) are invoked from Rust via synchronous
func.call (NAPI-reentrant context). Their synchronous catch blocks (and the
window-not-found early-return in setWindowDraggable) used bare hilog.warn/info,
which can throw 'Argc mismatch' and mask the original error (ohos-constraints 2.3).

Switched those sync paths to safeLogError. Async .then()/.catch() Promise
callbacks are left on hilog (not reentrant).
- window/mod.rs: bridge set_window_topmost/title/limits/ime_position/draggable/
  redraw/request_user_attention; instanceKey for specified launchType
- app.rs: notify_window_status NAPI queue + drain (windowStatusChange backfill)
- event.rs: FoldDisplayModeChange MainEvent
- ArkHelper.ets: ArkTS impls for the new window ops
- NativeAbility.ets + FloatPage.ets: windowStatusChange registration + seed;
  FloatPage title/min/max/close buttons, startMoving onTouch
- WindowManager.ets: showMainAbility(startAbility instanceKey=main),
  setPointerStyle real windowId, decoration flag interception (closable/
  maximizable/minimizable/resizable), setWindowTitle/Limits/Topmost
- type.ets: ArkHelper interface + UIAbilityWantParams.instanceKey

@ddxwzc-boop ddxwzc-boop left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

OHOS Code Review — openharmony-ability#45

🔴 🟡 🔵 ℹ️
0 1 3 1

Cross-repo group: window-ops bridge + status readback (with tao#20, tauri#73, plugins-workspace#23). The windowStatusChange readback chain (NativeAbility + FloatPage → notify_window_status → drain → apply_window_status) and the decoration-flag interception (问题四) are solid. Findings below. Inline comments attached.

Summary

  • 🟡 showMainAbility reuse path is non-functional as shipped: the tauri-cli template declares launchType: "standard" (= multiton, always-new-instance) and there is no onAcceptWant/AbilityStage. instanceKey is ignored under standard, so set_visible(true) on the main window after hide spawns a duplicate EntryAbility. The demo (demo/entry/.../module.json5) omits launchType → defaults to singleton → works there by coincidence, masking the bug in real tauri-generated apps.
  • 🔵 FloatPage isMaximized is local @State, not fed from windowStatusChange → maximize/restore button icon can desync from actual state.
  • 🔵 request_user_attention hardcodes notification id: 1 → concurrent calls across windows collide (second overwrites first).
  • 🔵 FloatPage replaced PanGesture with startMoving only (API14+) → Float dragging breaks on API<14 with no fallback. Acceptable if API floor is 14, but worth a guard.
  • ℹ️ New comments are in Chinese (H7) — 72 added lines. Note checklist-vs-idiom tension (surrounding OHOS code already uses Chinese).

Positive

  • getDecorationFlag interception in minimizeWindow/maximizeWindow/destroyWindow + FloatPage close check is a clean 问题四 fix.
  • setPointerStyle now uses getWindowProperties().id (real OHOS id) — corrects a prior silent no-op on main window.

this.hideAbility();
// Bring main Ability back to foreground via startAbility(instanceKey='main').
// onAcceptWant returns 'main' → system reuses existing instance (specified launchType).
showMainAbility(): void {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🟡 [F3/D] showMainAbility reuse is non-functional as shipped.

startAbility(want with instanceKey='main') only reuses an instance under launchType: "specified" + an AbilityStage.onAcceptWant() returning 'main'. Neither exists: the tauri-cli template declares launchType: "standard" (entry_desktop/entry_mobile module.json5:21), which is multiton — instanceKey is ignored and every startAbility creates a NEW EntryAbility. No onAcceptWant is implemented anywhere (grep-confirmed), and os.ets:20-21 notes the SDK doesn't support the abilityStage field in module.json5.

Net effect: set_visible(true) on the main window after hideWindow (now win.minimize()) spawns a duplicate main ability. The comments here (and at lines 472-473, 511) describing "onAcceptWant returns 'main' → system reuses existing instance (specified launchType)" describe a mechanism that doesn't exist.

Note: the demo/entry/.../module.json5 omits launchType → defaults to singleton, so the demo happens to work via onNewWant — masking this in dev. Real tauri-generated apps use the template's explicit standard and hit the duplicate.

Suggestion: either implement launchType: "specified" + AbilityStage/onAcceptWant (if the SDK allows, or via a different mechanism), or revert the main-window show path to a mechanism that works under standard/singleton (e.g. restore-from-minimize rather than startAbility). At minimum, correct the misleading comments.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in a052d3f. Reverted the main-window show path to restore() — the documented inverse of minimize() (the minimize() doc routes main-window restoration through restore(), sub-window through showWindow()). Guarded by isMinimized() so a redundant set_visible(true) on a visible maximized window won't un-maximize it. The AbilityStage/onAcceptWant path is out of reach (SDK 12 hvigor doesn't support the abilityStage field per os.ets:20-21), and startAbility(instanceKey='main') was spawning a duplicate under launchType: standard exactly as you noted, so it's removed. Misleading comments corrected; showMainAbility deleted; unused Want import dropped.

@LocalStorageProp('title') title: string = '';
@StorageProp("__openharmony_ability_is_desktop__") isDesktop: boolean = false;
// Maximize toggle state (local UI state, not persisted)
@State isMaximized: boolean = false;

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 [G9] isMaximized is local @State, not synced from windowStatusChange.

The maximize/restore button toggles isMaximized only on click. If the window is maximized/restored by the system or by the tao mirror (apply_window_status updates tao's maximized), the FloatPage button icon ( vs ) won't follow. G9 territory: mirror bit (tao side) is回灌 but the UI-side state is not.

Suggestion: drive isMaximized from the windowStatusChange callback (status MAXIMIZE/FLOATING) registered just above, or read getWindowStatus() in the callback to set isMaximized.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in a052d3f. isMaximized is now driven from the existing windowStatusChange callback (and seeded from getWindowStatus() in aboutToAppear), so isMaximized = (status === WindowStatusType.MAXIMIZE) tracks system/tao-mirror triggers too. Removed the optimistic isMaximized = true/false toggle from the button onClick — the callback is now the single source of truth for the icon.

try {
const doPublish = () => {
notificationManager.publish({
id: 1,

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 [impl] Hardcoded notification id: 1.

Concurrent request_user_attention calls (e.g. from multiple windows) share id: 1, so the second publish overwrites/suppresses the first. Also a fixed id means the notification can't be cancelled per-window later.

Suggestion: derive the id from the window id (e.g. id: windowId & 0xffff or a small per-window counter) so windows don't collide.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in a052d3f. Now uses a closure-level monotonic counter (++attentionNotifId) so concurrent calls get distinct ids instead of all sharing 1. Per-window id would be ideal, but tao's request_user_attention is called with no args (no windowId forwarded), so a monotonic id is the minimal change that kills the second-overwrites-first collision today.

.onTouch((event: TouchEvent) => {
if (event.type === TouchType.Down) {
try {
this.windowClass?.startMoving()?.catch((err: BusinessError) => {

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

🔵 [impl] PanGesture removed → drag depends solely on startMoving (API14+) with no fallback.

If startMoving is undefined/fails on API<14, Float sub-window dragging is broken (previously handled by PanGesture). Low impact if the API floor is 14, but there's no deviceInfo.sdkApiVersion guard (unlike setWindowTopmost).

Suggestion: guard startMoving by sdkApiVersion >= 14, or keep a PanGesture fallback for older devices.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Fixed in a052d3f. Added if (deviceInfo.sdkApiVersion < 14) return; before startMoving(), matching the sibling setWindowTopmost/restoreWindow guards (compatibleSdkVersion is API 12 here, so this path is real). No PanGesture fallback attached: a simultaneously-bound PanGesture consumes the Down event and blocks startMoving, so they can't coexist — Float dragging targets API14+ desktop. The guard keeps it a silent no-op (no per-touch-down warn, to avoid log spam; the sibling warns are single-call, not per-event).

…otif id, startMoving guard

Addresses the 4 findings in PR#45 review #4948263922.

F1 (🟡 main-window show non-functional under standard launchType):
  showMainAbility() did startAbility(instanceKey='main'), but instanceKey is
  ignored under launchType "standard" (tauri-cli template) → every call spawned a
  DUPLICATE EntryAbility. The demo masked this (omits launchType → singleton →
  onNewWant). Replaced the main-window show path with restore() — the documented
  inverse of minimize() (the minimize() doc routes main-window restoration through
  restore(), not showWindow()). Guarded by isMinimized() so a redundant
  set_visible(true) on a visible maximized window doesn't un-maximize it. Removed
  the now-unused Want import and the misleading onAcceptWant comments.

F2 (🔵 FloatPage isMaximized local @State desyncs from system/tao triggers):
  Drive isMaximized from the existing windowStatusChange callback (+ initial seed)
  instead of an optimistic button-click toggle, so the maximize/restore icon stays
  in sync when maximize/restore is triggered by the system or the tao mirror
  (apply_window_status). isMaximized = (status === WindowStatusType.MAXIMIZE).

F3 (🔵 request_user_attention hardcoded notif id:1 collides):
  Concurrent calls shared id:1 → second overwrote first. Now uses a closure-level
  monotonic counter so each call gets a unique id. (Signature kept no-arg: tao's
  request_user_attention does not forward windowId, so per-window id needs a
  full-chain param addition — monotonic id fixes the collision today.)

F4 (🔵 FloatPage startMoving API14+ with no guard):
  startMoving is API14+ but compatibleSdkVersion is API12 → on API<14 the method is
  undefined and threw on every touch-down. Added deviceInfo.sdkApiVersion<14 guard
  (matches sibling setWindowTopmost/restoreWindow). No PanGesture fallback attached:
  a simultaneously bound PanGesture consumes the Down event and blocks startMoving,
  so the two cannot coexist — drag targets API14+ desktop.
…indowId helper

set_cursor_grab resolves tao's window id to the real OHOS window id via a
synchronous getRealWindowId ArkTS helper, then calls
OH_WindowManager_LockCursor/UnlockCursor from libnative_window_manager.so.
The library is dlopen'd lazily (never statically linked — compatibleSdk is
API 12 and older images lack the symbols; dlsym-null doubles as the version
guard → NotSupported). Typed CursorGrabError {NotSupported, OsCode, Bridge}
lets tao map errors without string matching; unlock is idempotent
(1300002 after focus-loss auto-release maps to Ok). Also declares
LOCK_WINDOW_CURSOR in the HAR module.json5 (self-documenting; HAP-side
declaration is authoritative).
@ddxwzc-boop

Copy link
Copy Markdown
Author

Cursor grab FFI bridge appended (50d3f00)

Adds window::set_cursor_grab + getRealWindowId ArkTS helper:

  • getRealWindowId: synchronous single-arg NAPI call (same pattern as isMaximized), resolves tao's window id to the real OHOS window id via getWindowProperties().id; no hilog in the NAPI-called body (re-entrancy constraint), failure = -1 sentinel.
  • dlopen weak-loading: libnative_window_manager.so resolved lazily and cached in OnceLock — never statically linked (compatibleSdk is API 12; older images lack the symbols). dlsym-null is the version guard → NotSupported.
  • Typed CursorGrabError { NotSupported, OsCode(i32), Bridge(String) } so tao maps errors without string matching; unlock + 1300002 → Ok (idempotent; focus-loss auto-release makes repeat-unlock the common path).
  • LOCK_WINDOW_CURSOR added to the HAR module.json5 (self-documenting only; the HAP entry declaration in tauri#73 is authoritative).

Device-verified via tao#20 + tauri#73 (lock/unlock/focus-loss all pass, hilog clean).

…uard

0cac4c3 replaced WebViewStyle width/height with "100%" natural layout to
fix the main-webview resize relayout bug, which silently broke child
webview geometry: a child created at (x, y, w, h) rendered window-sized
at (x, y), so its bottom-right overflow was clipped by the window edge
(clipped amount == position offset).

Restore data.style.width/height in WebBuilder/EmbeddedWebBuilder, and add
a naturalLayout flag (set at creation when style.width is absent): natural
webviews — the main webview of each window — have runtime set_bounds
width/height stripped in updateWebviewStyle, keeping "100%" sizing so the
0cac4c3 fix does not regress. Explicit-bounds child webviews accept
width/height at creation and at runtime.

Verified on device (HUAWEI MateBook Pro 2in1): create_webview
multi-webview manual test renders the exact 300x200 @(50,50) rect.
/// fold display mode change event (folding screen fold/unfold)
/// alias display.on("foldDisplayModeChange")
/// https://developer.huawei.com/consumer/cn/doc/harmonyos-references/js-apis-display#onfolddisplaymodechange9
FoldDisplayModeChange(i32),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

这个功能没实现的吧,那就不要留着,删了

}
},

requestRedraw: (windowId: number): void => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

不支持的功能代码就都删了

@Eulogizethesun Eulogizethesun left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

OHOS Code Review — openharmony-ability#45(第二轮,回应 owner 意见)

🔴 🟡 🔵 ℹ️
0 3 2 0

两条 owner 意见(r3828608931 event.rs / r3828614326 ArkHelper.ets)均已逐链路核实为真,删除面已列在 inline comments 里;另按同一标准("不支持的功能代码就都删了")追加发现 instanceKey 也是同类死代码。此外 2 个 Minor(版本 guard 一致性、C5)。

本轮流程

Round 1 diff 扫描 + Round 2 subagent 深读(重点:上轮 review 后新增的 3 个 commit — cursor grab / webview bounds / ime readback)+ 对抗性验证(9 findings 中 4 条被质疑者反驳丢弃:BigInt 归一化前提不成立、showWindowMethod 非回归、lastImeResult 单例闭包无互串、doPublish 递归实际最多 2 层)。

跨仓提醒

F1/F2 的删除需要 tao#20 同步:删 request_redraw 的 import 与调用(tao 侧 request_redraw 可退回 {} no-op——平台 API 需保留占位)、删 MainEvent::FoldDisplayModeChange 的 match arm(oha 从未发射该事件,handler 永远不可达)。

Positive

  • cursor grab:dlopen/dlsym 惰性解析 + 符号缺失即版本守卫 + NotSupported 类型化错误映射,是高版本 NDK API 的正确接法
  • DefaultWebview naturalLayout 守卫精准修复了 0cac4c3 回归(主 webview 保持 100% 布局,子 webview 恢复显式 bounds)
  • 上轮 4 个 findings(restore 路径、isMaximized 同步、notif id、startMoving guard)修复到位

}
},

requestRedraw: (windowId: number): void => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🟡 [owner r3828614326 确认] requestRedraw 是 no-op log,应整链删除。

已核实全链无真实行为:ArkTS 实现只打 log(vsync 自动驱动);tao 侧此前是 {} no-op(tao#20 才接上这条桥,接的也是 no-op)。删除面:

  • 本处 + type.ets:197 接口声明
  • crates/ability/src/window/mod.rs:659 request_redraw
  • tao#20 同步:删 import 与调用,Window::request_redraw 退回 {}(跨平台 API 占位需保留,wry 侧无需感知)

/// fold display mode change event (folding screen fold/unfold)
/// alias display.on("foldDisplayModeChange")
/// https://developer.huawei.com/consumer/cn/doc/harmonyos-references/js-apis-display#onfolddisplaymodechange9
FoldDisplayModeChange(i32),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🟡 [owner r3828608931 确认] FoldDisplayModeChange 是死 enum,应删除。

已核实:全仓无任何构造点,ArkTS 侧也无 display.on('foldDisplayModeChange') 订阅——事件永远不会发射。tao#20 为它加的 MainEvent::FoldDisplayModeChange match arm 同样不可达,需一并删除。删除面:本变体 + event.rs:115 名字映射。

(若后续要做折叠屏适配,应从 ArkTS 订阅 + NAPI 发射开始,而不是先铺 enum。)

want.set("multiton", multiton)?;
want.set("transparent", transparent)?;
// instanceKey: unique per call → onAcceptWant returns unique key → new instance
want.set("instanceKey", format!("win-{}", window_id))?;

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🟡 [同类] instanceKey 也是"不支持的功能代码",建议一并删除。

按 owner 同一标准核实:tauri-cli 模板声明 launchType: "standard"(multiton,instanceKey 被系统忽略),SDK 12 不支持 module.json5 的 abilityStage 字段、无处实现 onAcceptWant——本 PR 的 a052d3f 修复里已确认该机制不存在并删除了 showMainAbility,却在 start_ui_ability 保留了同一参数。且 type.ets:143 的注释被改成 "new instance via launchType:specified",与模板实际值(standard)方向相反,与 a052d3f 修复注释自相矛盾。

删除面:本处 want.set("instanceKey", ...)type.ets:149 字段、ArkHelper.ets:539-543 透传,注释改回 standard。

try {
const win = WindowManager.getInstance().getWindow(windowId);
if (!win) { safeLogError('setWindowDraggable(window-not-found)', { code: -1, msg: `window ${windowId} not found` } as ESObject); return; }
win.enableDrag(enable).then(() => {

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🔵 [版本隔离] enableDrag 是 API 20+,但缺 deviceInfo.sdkApiVersion guard。

同 PR 的 setWindowTopmost(API14)、restoreWindow(API14)、startMoving(API14)都有 guard,这里是唯一例外。API<20 设备上 win.enableDrag 为 undefined → TypeError 被外层 catch 吃掉(不致命但每次调用报错)。建议 if (deviceInfo.sdkApiVersion < 20) { warn + return; } 对齐同 PR 模式。

try {
const realWindowId = win.getWindowProperties().id;
pointer.setPointerStyleSync(realWindowId, style as pointer.PointerStyle);
hilog.info(DOMAIN, 'WindowManager', 'setPointerStyle taoId=%{public}d realId=%{public}d style=%{public}d OK', windowId, realWindowId, style);

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

🔵 [C5] setPointerStyle 同步段使用裸 hilog,违反 NAPI 重入约束。

本方法经 Rust func.call 同步调用(NAPI 重入上下文),C5 规则明确此上下文 hilog 会抛 "Argc mismatch"(与格式串参数是否匹配无关)。同文件 getRealWindowId 的注释与写法是正确范例。后果:598 行成功日志抛出会把成功调用误记为 failed;599 行 catch 内 hilog 再抛会丢弃原始 err、掩盖真实错误。建议同步段去掉或用 safeLogError 包裹。

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