Open Bug 969762 Opened 6 years ago Updated 10 months ago

dump() doesn't support non-ASCII message on Windows

Categories

(Core :: DOM: Core & HTML, defect)

All
Windows 8.1
defect
Not set

Tracking

()

REOPENED

People

(Reporter: emk, Assigned: emk)

References

Details

Attachments

(1 file, 2 obsolete files)

Because the system code page is not UTF-8 on Windows.
Attached patch patch (obsolete) — Splinter Review
Try run: https://tbpl.mozilla.org/?tree=Try&rev=0c590924b049
Attachment #8372744 - Flags: review?(jmathies)
Attachment #8372744 - Attachment is patch: true
Can you add console service? 

http://mxr.mozilla.org/mozilla-central/source/xpcom/base/nsConsoleService.cpp#194

Also, outside the scope but it would be really cool to have a flag passed to PrintToDebugger indicating the source of the output.

I'd like to add a registry flag check to PrintToDebugger at some point that turns this on even when a debugger isn't present. Then we could use tools like DebugView to look at output without having to connect a debugger. We're using a flag like this with the command execute handler which has proven invaluable in debugging startup issues - 

http://mxr.mozilla.org/mozilla-central/source/browser/metro/shell/commandexecutehandler/CEHHelper.cpp#16
Attached patch patch v2 (obsolete) — Splinter Review
Try run: https://tbpl.mozilla.org/?tree=Try&rev=81c67a2d0071
Attachment #8372744 - Attachment is obsolete: true
Attachment #8372744 - Flags: review?(jmathies)
Attachment #8373045 - Flags: review?(jmathies)
Comment on attachment 8373045 [details] [diff] [review]
patch v2

Review of attachment 8373045 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/base/Debug.cpp
@@ +52,5 @@
> +#endif
> +
> +#ifndef XP_WIN
> +    if (!(aOptions & kPrintToStream)) {
> +      return;

Oops, I forgot nsMemory::Free(cstr); here. I'll fix this before landing.
Attached patch patch v3Splinter Review
- Added a documentation to the option flags.
- Stop using a raw pointer so that we can't forget to free the memory.
Attachment #8373045 - Attachment is obsolete: true
Attachment #8373045 - Flags: review?(jmathies)
Attachment #8373318 - Flags: review?(jmathies)
Attachment #8373318 - Flags: review?(jmathies) → review+
https://hg.mozilla.org/integration/mozilla-inbound/rev/d7d447cac91b
Assignee: nobody → VYV03354
Status: NEW → ASSIGNED
Comment on attachment 8373318 [details] [diff] [review]
patch v3

>+void mozilla::PrintToDebugger(const nsAString& aStr, FILE* aStream,
>+                              LogOptions aOptions)
>+{
>+  nsString msg(aStr);
>+  if (aOptions & kPrintNewLine) {
>+    msg.AppendLiteral("\n");
>+  }
I've always wanted to limit the output of OutputDebugString, as WinDbg hangs if you feed it very long lines. Assuming that this is the only remaining call, this gives me a nice central spot to do this, but I don't know whether it would be generally useful.
https://hg.mozilla.org/mozilla-central/rev/d7d447cac91b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla30
Backed out due to a huge tp5 regression.
https://hg.mozilla.org/integration/mozilla-inbound/rev/47e7d1ed3ab4
Status: RESOLVED → REOPENED
Depends on: 971167
Resolution: FIXED → ---
Merge of backout:
https://hg.mozilla.org/mozilla-central/rev/47e7d1ed3ab4
Target Milestone: mozilla30 → ---
Blocks: 972250
Component: DOM → DOM: Core & HTML
You need to log in before you can comment on or make changes to this bug.