336684 - [FIX]Atomize strings if passed to an nsIAtom param.

Status: RESOLVED FIXED

(Core :: XPConnect, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

I have a fix for this... but I have no idea how to test it.  Can someone point me to a JS-exposed API that takes nsIAtom in params?

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: Fix

Comment 2

[[e-mail]]

(In reply to comment #0)
>Can someone point me to a JS-exposed API that takes nsIAtom in params?
With this patch the functions in editorUtilities.js work using strings.

Comment 3

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

Wouldn't you want to convert to a string even something other then a string is passed? That seems more ecmascript-y.

Comment 4

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

But not very XPConnecty: we don't coerce objects to strings (or strings to objects, even, though ECMA might have us do that where an object was wanted) when calling a method that expects one.  String(myObj) is not too burdensome, in exchange for less surprising behaviour for callers.

Comment 5

[Boris Zbarsky [:bzbarsky]]

> Wouldn't you want to convert to a string even something other then a string is
> passed?

That would break passing in actual nsIAtom objects, since they'd get stringified to "[XPConnect wrapped nsIAtom]" or whatever, which would then get atomized.  So no, I don't want to do that.  ;)

Neil, thanks for testing this!

Comment 6

[[e-mail]]

(In reply to comment #5)
>>Wouldn't you want to convert to a string even something other then a string is
>>passed?
>That would break passing in actual nsIAtom objects, since they'd get
>stringified to "[XPConnect wrapped nsIAtom]" or whatever, which would then get
>atomized.  So no, I don't want to do that.  ;)
Perhaps if nsIAtom objects were made to stringify to their value ;-)

Comment 7

[Boris Zbarsky [:bzbarsky]]

Then it would break on objects which have an nsIAtom on the proto chain and override toString()...  And so forth.  I'd really like to do exactly what this patch does, frankly.

Comment 8

[Johnny Stenback (:jst)]

Comment on attachment 220873 [details] [diff] [review]
Fix

r=jst

Comment 9

[Mike Shaver (:shaver -- probably not reading bugmail closely)]

Comment on attachment 220873 [details] [diff] [review]
Fix

sr=shaver

Comment 10

[Boris Zbarsky [:bzbarsky]]

Fixed.  We should document this somehow, but I'm not sure how.