Closed
Bug 336684
Opened 18 years ago
Closed 18 years ago
[FIX]Atomize strings if passed to an nsIAtom param.
Categories
(Core :: XPConnect, defect, P2)
Tracking
()
RESOLVED
FIXED
mozilla1.9alpha1
People
(Reporter: bzbarsky, Assigned: bzbarsky)
Details
Attachments
(1 file)
2.35 KB,
patch
|
jst
:
review+
shaver
:
superreview+
|
Details | Diff | Splinter Review |
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?
Assignee | ||
Comment 1•18 years ago
|
||
Attachment #220873 -
Flags: superreview?(shaver)
Attachment #220873 -
Flags: review?(jst)
Comment 2•18 years ago
|
||
(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.
Wouldn't you want to convert to a string even something other then a string is passed? That seems more ecmascript-y.
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.
Assignee | ||
Comment 5•18 years ago
|
||
> 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•18 years ago
|
||
(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 ;-)
Assignee | ||
Comment 7•18 years ago
|
||
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•18 years ago
|
||
Comment on attachment 220873 [details] [diff] [review] Fix r=jst
Attachment #220873 -
Flags: review?(jst) → review+
Comment on attachment 220873 [details] [diff] [review] Fix sr=shaver
Attachment #220873 -
Flags: superreview?(shaver) → superreview+
Assignee | ||
Comment 10•18 years ago
|
||
Fixed. We should document this somehow, but I'm not sure how.
Status: NEW → RESOLVED
Closed: 18 years ago
Priority: -- → P2
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9alpha
You need to log in
before you can comment on or make changes to this bug.
Description
•