nsDebugImpl's Break() passes bogus wide string to windbgdlg.exe

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: vlad, Assigned: neil)

Tracking

({fixed1.9.1})

Trunk
x86
Windows XP
Points:
---
Dependency tree / graph
Bug Flags:
blocking1.9.1 +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 1 obsolete attachment)

This was changed in bug 455381 in the mass change to using the W versions of win32 functions.  However, windbgdlg.exe is ours (in xpcom/windbgdlg), and it treats the cmd line as a C string, not a wide string.  We're now converting to a wide string or something before launching in http://hg.mozilla.org/mozilla-central/annotate/f90c51ed3cd2/xpcom/base/nsDebugImpl.cpp#l400 .  I'm actually not sure what's going on, the net result is that all messages come out as chinese characters instead of correct in the abort/quit/debug dialog box.

Marking as blocking, because there's a potential for subtle corruption here.
Flags: blocking1.9.1+
Assignee

Comment 2

11 years ago
Comment on attachment 346946 [details] [diff] [review]
removed nsCommonProcess 

>-    wsprintf(msg,
>-             "%s\n\nClick Abort to exit the Application.\n"
>-             "Click Retry to Debug the Application..\n"
>-             "Click Ignore to continue running the Application.", 
>+    wsprintfW(msg,
>+             L"%s\n\nClick Abort to exit the Application.\n"
>+             L"Click Retry to Debug the Application..\n"
>+             L"Click Ignore to continue running the Application.", 
>              lpszCmdLine);
What a shame lpszCmdLine is still narrow...
Assignee

Comment 3

11 years ago
I'm not sure this works because I've heard that not everyone supports wWinMain.
Assignee: blassey → neil
Status: NEW → ASSIGNED
Attachment #351523 - Flags: superreview?(benjamin)
Attachment #351523 - Flags: review?(blassey)
Assignee

Comment 4

11 years ago
Posted patch Option 2: reuse lpszCmdLineW (obsolete) — Splinter Review
Attachment #351525 - Flags: superreview?(benjamin)
Attachment #351525 - Flags: review?(blassey)
Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

I think the switch to wWinMain is preferable since we don't support win95/98 anymore.
Attachment #351523 - Flags: review?(blassey) → review+
...and thank you for fixing the spacing

Updated

11 years ago
Attachment #351523 - Flags: superreview?(benjamin) → superreview+
Assignee

Comment 7

11 years ago
Pushed changeset 79c023857355 to mozilla-central.

(Actually it was some hours ago, I just forgot to update the bug until now...)
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED

Updated

11 years ago
Attachment #351525 - Flags: superreview?(benjamin)
Attachment #351525 - Attachment is obsolete: true
Attachment #351525 - Flags: review?(bugmail)
Assignee

Updated

11 years ago
Attachment #351523 - Flags: approval1.9.1?
Assignee

Comment 8

11 years ago
Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

Needed to fix regression from bug 455381.
Comment on attachment 351523 [details] [diff] [review]
Option 1: really make everything wide

Blocker, doesn't need approval.
Attachment #351523 - Flags: approval1.9.1?
Whiteboard: [needs 1.9.1 landing]
Assignee

Comment 10

11 years ago
(In reply to comment #9)
> Blocker, doesn't need approval.
Thanks for pointing that out.

Pushed changeset f8aedd7dc4c7 to releases/mozilla1.9.1
Assignee

Updated

11 years ago
Keywords: fixed1.9.1
Whiteboard: [needs 1.9.1 landing]
You need to log in before you can comment on or make changes to this bug.