Fix padded Wine review strings

tests/README.md

@@ -8,6 +8,15 @@ text, Unicode offsets before the viewport, wrapping/blank lines, an empty
 focus anchor, hidden/password exclusion, password transitions before copying,
 clipping, same-length Edit/ListBox/ListView races, native ListView cells,
 negative coordinates, source limits and unchanged focus/selection.
+It also covers output serviced by a 16 ms message loop, late scalar replies,
+nested native containers, deeply clipped text, and maximized versus side-by-side
+MDI documents. Covered documents are excluded while visible titles remain
+available. These checks reproduce capture failures that can make review report
+that it cannot find the current location despite valid Wine focus.
+The padded MSAA status-name case checks that text after its terminating NUL
+does not reach D-Bus and that range lengths match the text actually sent.
+A local provider also checks that malformed UTF-16 padding cannot hide the
+valid label.
 
 Run `python3 tests/review_integration.py --build-dir build` and its `--32-bit`
 variant sequentially. These exercise the exact wire types, actual-owner

tests/review_capture_test.cpp

@@ -9,6 +9,17 @@
 #include "../review_capture.h"
 #define HAS_CAPTURE 1
 #endif
+static LRESULT CALLBACK padded_name_proc(HWND window,UINT message,WPARAM wp,LPARAM lp) {
+  const WCHAR text[]={L'R',L'e',L'a',L'd',L'y',0,0xd800,0};
+  if(message==WM_GETTEXTLENGTH)return 7;
+  if(message==WM_GETTEXT && wp) {
+    const size_t copied=std::min<size_t>(7,wp-1);
+    std::copy_n(text,copied,reinterpret_cast<WCHAR*>(lp));
+    reinterpret_cast<WCHAR*>(lp)[copied]=0;
+    return copied;
+  }
+  return DefWindowProcW(window,message,wp,lp);
+}
 
 static bool check(bool value, const char *message) {
   if (!value) std::fprintf(stderr, "FAIL: %s\n", message);
@@ -67,15 +78,18 @@ int main(int argc, char **argv) {
     ok &= check(text.find("private-review-secret") == std::string::npos && text.find("hidden-review-secret") == std::string::npos,
                 "password and hidden contents excluded");
     ok &= check(SendMessageW(window, WM_APP + 11, 0, 0), "review preserves focus and selection");
-    for (int mode : {1, 2, 3, 4, 5, 6, 7, 8, 9, 14, 15, 17, 18, 21, 22, 23, 24, 25}) {
+    for (int mode : {1, 2, 3, 4, 5, 6, 7, 8, 9, 14, 15, 17, 18, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33}) {
       std::fprintf(stderr, "capture test: mode %d\n", mode);
       SendMessageW(window, WM_APP + 10, mode, 0);
+      auto started=GetTickCount64();
       auto result = capture();
+      std::fprintf(stderr,"capture test: status=%s elapsed=%llu ms\n",result.status.c_str(),static_cast<unsigned long long>(GetTickCount64()-started));
       const auto value = joined(result);
       if (mode == 7) ok &= check(result.status == "limited" && result.nodes.empty(), "source limit rejects partial capture");
       else if (mode == 9) ok &= check(result.status == "stale" && result.nodes.empty(), "same-length update during capture rejects mixed data");
       else if (mode == 14 || mode == 15 || mode == 18 || mode == 23) ok &= check(result.status == "stale" && result.nodes.empty(), "protection/list/viewport transition rejects old capture");
       else if (mode == 21) ok &= check(result.status != "ok" && result.nodes.empty(), "delayed native call respects capture deadline");
+      else if (mode == 27) ok &= check(result.status != "ok" && result.nodes.empty() && GetTickCount64()-started<600,"delayed scalar batch respects capture deadline");
       else ok &= check(result.status == "ok", "configured capture succeeds");
       ok &= check(value.find("private-review-secret") == std::string::npos && value.find("hidden-review-secret") == std::string::npos,
                   "all captures exclude protected/hidden text");
@@ -102,6 +116,37 @@ int main(int argc, char **argv) {
       if (mode == 14) ok &= check(SendMessageW(window,WM_APP + 14,0,0), "password transition is excluded before copying text");
       if (mode == 17) ok &= check(value.find("table & item") != std::string::npos && value.find("ready") != std::string::npos, "visible native ListView cells retain text and ampersands");
       if (mode == 25) ok &= check(value.find("literal & label") != std::string::npos, "SS_NOPREFIX captions retain literal ampersands");
+      if (mode == 26) {
+        size_t visible_lines=0;
+        for(const auto &node:result.nodes)for(const auto &range:node.ranges)
+          if(range.text=="stable buffered output line")++visible_lines;
+        ok &= check(visible_lines>=15,"frame-pumped multiline output captures within the deadline");
+      }
+      if(mode==27){
+        Sleep(750);SendMessageW(window,WM_APP+10,0,0);
+        auto recovered=capture();
+        ok &= check(recovered.status=="ok" && joined(recovered).find("alpha beta")!=std::string::npos,"late scalar callbacks preserve storage and allow a fresh capture");
+      }
+      if(mode==28)ok &= check(value.find("top clipped visible line")!=std::string::npos,"top-clipped Edit captures its lower visible rows");
+      if(mode==29)ok &= check(value.find("nested output ready")!=std::string::npos && SendMessageW(window,WM_APP+16,0,0),"native containers do not synthesize duplicate accessibility trees");
+      if(mode==30)ok &= check(value.find("active mdi text")!=std::string::npos && value.find("covered mdi text")==std::string::npos,"maximized MDI child excludes covered background text");
+      if(mode==31)ok &= check(value.find("active mdi text")!=std::string::npos && value.find("covered mdi text")!=std::string::npos,"side-by-side MDI children retain both visible documents");
+      if (mode == 31) ok &= check(value.find("Active") != std::string::npos && value.find("Background") != std::string::npos,
+                                 "side-by-side MDI window titles remain available for review");
+      if (mode == 32) ok &= check(value.find("deep clipped visible line") != std::string::npos,
+                                 "hundreds of clipped rows do not hide the visible tail");
+      if (mode == 33) {
+        bool safe=true, status_present=false;
+        const auto status_id=store.identity(GetDlgItem(window,114))+"_a";
+        for(const auto &node:result.nodes) {
+          if(node.id==status_id)status_present=true;
+          safe &= node.name.find('\0')==std::string::npos;
+          for(const auto &range:node.ranges)safe &= range.text.find('\0')==std::string::npos;
+        }
+        ok &= check(status_present && safe,"padded MSAA status name produces D-Bus-safe accessibility text");
+        ok &= check(value.find("Ready")!=std::string::npos,
+                    "status bar retains its exposed label");
+      }
       if (mode == 21) {
         Sleep(750);
         ok &= check(SendMessageW(window,WM_APP + 15,0,0), "remote buffer lives until the delayed target returns");
@@ -128,6 +173,23 @@ int main(int argc, char **argv) {
       }
       if (mode != 4) ok &= check(SendMessageW(window, WM_APP + 11, 0, 0), "capture leaves caret and focus unchanged");
     }
+    // A local provider avoids cross-process WM_GETTEXT marshalling, which
+    // itself truncates at the first NUL. Its BSTR retains malformed padding.
+    WNDCLASSW local{};local.hInstance=GetModuleHandleW(nullptr);
+    local.lpfnWndProc=DefWindowProcW;local.lpszClassName=L"WineReviewLocalRoot";
+    RegisterClassW(&local);
+    HWND local_root=CreateWindowW(local.lpszClassName,L"Padded MSAA name",WS_OVERLAPPEDWINDOW,
+        100,100,400,200,nullptr,nullptr,local.hInstance,nullptr);
+    local.lpfnWndProc=padded_name_proc;local.lpszClassName=L"WineReviewPaddedName";
+    RegisterClassW(&local);
+    HWND padded=CreateWindowW(local.lpszClassName,L"",WS_CHILD|WS_VISIBLE,
+        10,10,200,30,local_root,nullptr,local.hInstance,nullptr);
+    ShowWindow(local_root,SW_SHOW);SetForegroundWindow(local_root);SetFocus(padded);
+    auto padded_focus=store.observe_foreground();
+    auto padded_capture=capture_review_window(store,padded_focus,GetTickCount64()+500);
+    ok &= check(padded_capture.status=="ok" && joined(padded_capture).find("Ready")!=std::string::npos,
+                "malformed UTF-16 padding does not discard the valid MSAA label");
+    DestroyWindow(local_root);
     CoUninitialize();
 #endif
     PostMessageW(window, WM_CLOSE, 0, 0);

tests/review_fixture.c

@@ -11,6 +11,7 @@
 #define REVIEW_PRIVACY_CHECK (WM_APP + 14)
 #define REVIEW_REMOTE_CHECK (WM_APP + 15)
 static HWND editor, output, password, hidden;
+static HWND mdi, mdi_active, mdi_background;
 static WCHAR *large;
 static WNDPROC original_edit;
 static BOOL replace_during_read;
@@ -24,6 +25,27 @@ static WNDPROC original_list;
 static WNDPROC original_table;
 static BOOL replace_table_during_read;
 static BOOL delay_table_read, freed_table_memory, scroll_during_position;
+static BOOL frame_pump;
+static BOOL delay_batch;
+static unsigned int synthetic_containers;
+static LRESULT CALLBACK status_proc(HWND window,UINT message,WPARAM wp,LPARAM lp) {
+  // Like MUSHclient's status bar, report a length beyond the first NUL.
+  // The standard MSAA fallback retains that padding in its BSTR name.
+  const WCHAR text[]=L"Ready\0Connected";
+  if(message==WM_GETTEXTLENGTH)return (sizeof(text)/sizeof(*text))-1;
+  if(message==WM_GETTEXT && wp) {
+    size_t copied=(sizeof(text)/sizeof(*text))-1;
+    if(copied>wp-1)copied=wp-1;
+    memcpy((void*)lp,text,copied*sizeof(*text));
+    ((WCHAR*)lp)[copied]=0;
+    return copied;
+  }
+  return DefWindowProcW(window,message,wp,lp);
+}
+static LRESULT CALLBACK container_proc(HWND window,UINT message,WPARAM wp,LPARAM lp) {
+  if(message==WM_GETOBJECT && lp==OBJID_QUERYCLASSNAMEIDX){++synthetic_containers;Sleep(25);}
+  return DefWindowProcW(window,message,wp,lp);
+}
 static LRESULT CALLBACK table_proc(HWND window, UINT message, WPARAM wp, LPARAM lp) {
   if (message == LVM_GETITEMTEXTW && delay_table_read) {
     delay_table_read=FALSE;
@@ -59,6 +81,7 @@ static LRESULT CALLBACK stall_proc(HWND window, UINT message, WPARAM wp, LPARAM
   return DefWindowProcW(window,message,wp,lp);
 }
 static LRESULT CALLBACK edit_proc(HWND window, UINT message, WPARAM wp, LPARAM lp) {
+  if(message==EM_LINEINDEX && delay_batch){delay_batch=FALSE;Sleep(650);}
   if (message == WM_GETTEXT && (GetWindowLongPtrW(window,GWL_STYLE) & ES_PASSWORD)) ++protected_text_reads;
   if (message == WM_KEYDOWN) ++key_down;
   if (message == WM_KEYUP) ++key_up;
@@ -87,6 +110,12 @@ static LRESULT CALLBACK window_proc(HWND window, UINT message, WPARAM wp, LPARAM
     return 0;
   }
   if (message == REVIEW_CONFIG) {
+    frame_pump=FALSE;
+    delay_batch=FALSE;
+    synthetic_containers=0;
+    ShowWindow(GetDlgItem(window,112),SW_HIDE);
+    ShowWindow(GetDlgItem(window,114),SW_HIDE);
+    ShowWindow(mdi,SW_HIDE);
     SendMessageW(editor,EM_SETPASSWORDCHAR,0,0);
     protect_during_length = replace_list_during_read = FALSE;
     replace_table_during_read=FALSE;
@@ -105,6 +134,55 @@ static LRESULT CALLBACK window_proc(HWND window, UINT message, WPARAM wp, LPARAM
     SendMessageW(editor, EM_SETSEL, 0, 0);
     MoveWindow(editor, 20, 35, 320, 90, TRUE);
     switch (wp) {
+    case 33: ShowWindow(GetDlgItem(window,114),SW_SHOW); break;
+    case 32: {
+      WCHAR text[16384]; unsigned used = 0;
+      for (unsigned row = 0; row < 400; ++row) {
+        const WCHAR line[] = L"deep clipped visible line\r\n";
+        for (unsigned j = 0; j < (sizeof(line)/sizeof(*line))-1; ++j) text[used++] = line[j];
+      }
+      text[used] = 0;
+      SetWindowTextW(editor, text);
+      MoveWindow(editor, 20, -5000, 320, 5100, TRUE);
+      break;
+    }
+    case 30:
+      ShowWindow(mdi,SW_SHOW);
+      SendMessageW(mdi,WM_MDIACTIVATE,(WPARAM)mdi_active,0);
+      SendMessageW(mdi,WM_MDIMAXIMIZE,(WPARAM)mdi_active,0);
+      break;
+    case 31:
+      ShowWindow(mdi,SW_SHOW);
+      SendMessageW(mdi,WM_MDIRESTORE,(WPARAM)mdi_active,0);
+      SendMessageW(mdi,WM_MDIRESTORE,(WPARAM)mdi_background,0);
+      MoveWindow(mdi_background,0,0,240,300,TRUE);
+      MoveWindow(mdi_active,245,0,240,300,TRUE);
+      SendMessageW(mdi,WM_MDIACTIVATE,(WPARAM)mdi_active,0);
+      break;
+    case 28: {
+      WCHAR text[4096];unsigned used=0;
+      for(unsigned row=0;row<80;++row) {
+        const WCHAR line[]=L"top clipped visible line\r\n";
+        for(unsigned j=0;j<(sizeof(line)/sizeof(*line))-1;++j)text[used++]=line[j];
+      }
+      text[used]=0;SetWindowTextW(editor,text);
+      MoveWindow(editor,20,-250,320,380,TRUE);
+      break;
+    }
+    case 29:ShowWindow(GetDlgItem(window,112),SW_SHOW);break;
+    case 27:delay_batch=TRUE;break;
+    case 26: {
+      WCHAR text[4096];unsigned used=0;
+      for(unsigned row=0;row<80;++row) {
+        const WCHAR line[]=L"stable buffered output line\r\n";
+        for(unsigned j=0;j<(sizeof(line)/sizeof(*line))-1;++j)text[used++]=line[j];
+      }
+      text[used]=0;SetWindowTextW(editor,text);
+      MoveWindow(editor,20,35,320,380,TRUE);
+      ShowWindow(output,SW_HIDE);ShowWindow(GetDlgItem(window,106),SW_HIDE);
+      frame_pump=TRUE;
+      break;
+    }
     case 1: // surrogate before first displayed row, absolute offsets
       SetWindowTextW(editor, L"\U0001f600alpha\r\nbeta\r\nlast");
       SendMessageW(editor, EM_LINESCROLL, 0, 1);
@@ -166,6 +244,7 @@ static LRESULT CALLBACK window_proc(HWND window, UINT message, WPARAM wp, LPARAM
     return 1;
   }
   if (message == REVIEW_PRIVACY_CHECK) return protected_text_reads == 0;
+  if (message == WM_APP+16) return synthetic_containers==0;
   if (message == REVIEW_REMOTE_CHECK) return !freed_table_memory;
   if (message == REVIEW_CHECK || message == REVIEW_CHECK_RIGHT || message == REVIEW_CHECK_STRICT) {
     DWORD start = 0, end = 0;
@@ -243,11 +322,31 @@ int WINAPI WinMain(HINSTANCE instance, HINSTANCE previous, LPSTR args, int show)
   CreateWindowW(L"Static",L"literal & label",WS_CHILD|SS_NOPREFIX,
       20,365,200,20,window,(HMENU)111,instance,NULL);
   CreateWindowW(stall.lpszClassName,L"",WS_CHILD|WS_VISIBLE,350,150,150,20,window,NULL,instance,NULL);
+  WNDCLASSW container={0};container.lpfnWndProc=container_proc;container.hInstance=instance;container.lpszClassName=L"WineAccessNativeContainer";
+  RegisterClassW(&container);
+  HWND parent=CreateWindowW(container.lpszClassName,L"",WS_CHILD,20,35,320,100,window,(HMENU)112,instance,NULL);
+  for(unsigned level=0;level<8;++level)parent=CreateWindowW(container.lpszClassName,L"",WS_CHILD|WS_VISIBLE,0,0,320,100,parent,NULL,instance,NULL);
+  CreateWindowW(L"Edit",L"nested output ready",WS_CHILD|WS_VISIBLE|ES_READONLY,0,0,320,80,parent,NULL,instance,NULL);
+  WNDCLASSW mdi_child={0};mdi_child.lpfnWndProc=DefMDIChildProcW;mdi_child.hInstance=instance;mdi_child.lpszClassName=L"WineAccessMdiChild";
+  RegisterClassW(&mdi_child);
+  CLIENTCREATESTRUCT client_create={NULL,200};
+  mdi=CreateWindowW(L"MDIClient",L"",WS_CHILD|WS_CLIPCHILDREN,20,35,500,350,window,(HMENU)113,instance,&client_create);
+  mdi_background=CreateMDIWindowW(mdi_child.lpszClassName,L"Background",WS_OVERLAPPEDWINDOW|WS_VISIBLE,0,0,240,300,mdi,instance,0);
+  CreateWindowW(L"Edit",L"covered mdi text",WS_CHILD|WS_VISIBLE|ES_READONLY,0,0,220,100,mdi_background,NULL,instance,NULL);
+  mdi_active=CreateMDIWindowW(mdi_child.lpszClassName,L"Active",WS_OVERLAPPEDWINDOW|WS_VISIBLE,245,0,240,300,mdi,instance,0);
+  CreateWindowW(L"Edit",L"active mdi text",WS_CHILD|WS_VISIBLE|ES_READONLY,0,0,220,100,mdi_active,NULL,instance,NULL);
+  WNDCLASSW status=cls;status.lpszClassName=L"WineAccessStatusFixture";status.lpfnWndProc=status_proc;
+  RegisterClassW(&status);
+  CreateWindowW(status.lpszClassName,L"",WS_CHILD,20,420,480,20,window,(HMENU)114,instance,NULL);
   ShowWindow(window, SW_SHOW);
   SendMessageW(window, REVIEW_CONFIG, 0, 0);
   if (secondary) SetTimer(window,1,400,NULL);
   MSG message;
-  while (GetMessageW(&message, NULL, 0, 0) > 0) {
+  for (;;) {
+    if (frame_pump) {
+      if (!PeekMessageW(&message,NULL,0,0,PM_REMOVE)) { Sleep(16);continue; }
+      if (message.message==WM_QUIT)break;
+    } else if (GetMessageW(&message,NULL,0,0)<=0)break;
     TranslateMessage(&message); DispatchMessageW(&message);
   }
   return 0;

tests/review_integration.py

@@ -115,6 +115,20 @@ with tempfile.TemporaryDirectory(prefix='wine-review-wire-') as temporary:
         assert isinstance(wrong[0],GLib.Error) and 'AccessDenied' in str(wrong[0])
         other.close_sync(None)
         print('PASS: test_wrong_caller')
+        configure(33)
+        reply,_ = capture(); data=reply.unpack()[0]
+        assert data['status']=='ok', data['status']
+        ready=False
+        for node in data['nodes']:
+            assert '\0' not in node['name']
+            for start,end,native_start,native_end,text,bounds in node['ranges']:
+                assert '\0' not in text
+                assert end-start==len(text)
+                assert native_end-native_start==len(text.encode('utf-16-le'))//2
+                if text=='Ready': ready=True
+        assert ready, ('Padded MSAA status name lost its exposed text', data)
+        subprocess.run(['wine',str(fixture),'--check'],env=env,check=True,timeout=5)
+        print('PASS: test_padded_status_name_wire_lengths')
         configure(9)
         reply,_ = capture(); data=reply.unpack()[0]
         assert data['status'] in ('ok','stale')