Comment on attachment 638153
attribute as DOMString
Review of attachment 638153:
-----------------------------------------------------------------
::: widget/nsIBaseWindow.idl
@@ +156,5 @@
> + This is the handle (HWND, GdkWindow*, ...) to the native window of the
> + control, exposed as a DOMString.
> +
> + @return DOMString in hex format with "0x" prepended, or null if mainWidget
> + undefined
It seems you don't actually return null, but an empty string if there is no native window.
It might be better to simply return an empty string for all failure conditions. I'm not sure it's worth trying to distinguish different failure conditions.
::: widget/tests/test_bug760802.html
@@ +57,5 @@
> + "nativeHandle should not be implemented for nsDocShell"
> +);
> +
> +SimpleTest.ok(typeof(nativeHandle) === "string", "nativeHandle should be a string");
> +SimpleTest.ok(nativeHandle.match(/^0x[0-9a-f]+/), "nativeHandle should have a memory address format");
You don't need to prefix ok() and is() with SimpleTest, do you?
Have you tested this on Windows? Visual C++ produces capital hex digits for %p. Maybe you should put a tolower method call somewhere so only lower-case digits are returned by this API.
Comment on attachment 638153
attribute as DOMString
Review of attachment 638153: ------- ------- ------- ------- ------- ------- ------- ------- --
-------
::: widget/ nsIBaseWindow. idl
@@ +156,5 @@
> + This is the handle (HWND, GdkWindow*, ...) to the native window of the
> + control, exposed as a DOMString.
> +
> + @return DOMString in hex format with "0x" prepended, or null if mainWidget
> + undefined
It seems you don't actually return null, but an empty string if there is no native window.
It might be better to simply return an empty string for all failure conditions. I'm not sure it's worth trying to distinguish different failure conditions.
::: widget/ tests/test_ bug760802. html ok(typeof( nativeHandle) === "string", "nativeHandle should be a string"); ok(nativeHandle .match( /^0x[0- 9a-f]+/ ), "nativeHandle should have a memory address format");
@@ +57,5 @@
> + "nativeHandle should not be implemented for nsDocShell"
> +);
> +
> +SimpleTest.
> +SimpleTest.
You don't need to prefix ok() and is() with SimpleTest, do you?
Have you tested this on Windows? Visual C++ produces capital hex digits for %p. Maybe you should put a tolower method call somewhere so only lower-case digits are returned by this API.
::: xpfe/appshell/ src/nsXULWindow .cpp UNEXPECTED; toUTF16( addrStr) ;
@@ +753,5 @@
> + |Number| for instance) */
> + char* addrStr = PR_smprintf("0x%p", nativeWindowPtr);
> + if (!addrStr)
> + return NS_ERROR_
> + aNativeHandle = NS_ConvertASCII
You could use nsPrintfCString instead for slightly simpler code (and avoiding an error path).