Open Bug 462508 Opened 16 years ago Updated 3 years ago

nsPrimitiveHelpers::CreateDataFromPrimitive will fail to return a string for most nsISupportsCString

Categories

(Core :: DOM: Copy & Paste and Drag & Drop, defect, P5)

x86
Linux
defect

Tracking

()

UNCONFIRMED

People

(Reporter: tomeu, Unassigned)

Details

Attachments

(1 file)

User-Agent:       Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.0.3) Gecko/2008092510 Ubuntu/8.04 (hardy) Firefox/3.0.3
Build Identifier: xulrunner 1.9

Using nsIClipboardDragDropHooks, my embedding application would like to add a UTF-8 selection to an image that is being dragged away.

Unfortunately, nsPrimitiveHelpers::CreateDataFromPrimitive will only supply back an UTF-16-encoded string if nsIFlavorDataProvider returned a nsISupportString or an UTF-8-encoded string if nsIFlavorDataProvider returned a nsISupportCString _and_ the flavor is kTextMime.

Fro embedders to be able to add UTF-8-encoded selection values, I would like to change that so that if the nsIFlavorDataProvider returned a nsISupportCString, return an UTF-8-encoded string.

Reproducible: Always

Steps to Reproduce:
1.
2.
3.
Attachment #345702 - Flags: review? → review?(roc)
Wouldn't it be simpler to completely ignore the flavor (and remove that parameter) and just check whether aPrimitive QIs to nsISupportsCString?
(In reply to comment #2)
> Wouldn't it be simpler to completely ignore the flavor (and remove that
> parameter) and just check whether aPrimitive QIs to nsISupportsCString?

Would work for me, but that code is called from several platform-dependant places, so I'm not very comfortable making such big changes to mozilla.

I wonder if some nsIFlavorDataProvider could return something that implements both nsISupportsCString and nsISupportsString, in that case the flavor could be the hint that helps choose the best string type to return. But I'm just guessing here...

Regardless, I volunteer to do the grunt work if someone else can verify it's safe to do so.
It seems OK to me. Enndeakin, what do you think?
The callers of that method generally assume that text/plain returns single-byte and that text/unicode returns double-byte strings, and do static_casts on the results using that assumption. So the patch here I think isn't sufficient without fixing up the callers as well.

For now we should just do whatever is easiest. In the next round, I plan to convert the clipboard apis to use dataTransfer like I did for drag and drop, and then just have both use the dataTransfer directly, eliminating the tranferable and this nsPrimitiveHelpers class entirely.
Thanks. Then I'm not sure what's the right thing to do here. Tomeu, what's your MIME type here?
(In reply to comment #6)
> Thanks. Then I'm not sure what's the right thing to do here. Tomeu, what's your
> MIME type here?

Not really a mime type, we are adding some selections for passing metadata about the clipping that is displayed in Sugar's clipboard manager. We'd prefer these selections to be in UTF-8, as I understand that it's the preferred encoding for selections that contain text (we'd like these properties to be added to some spec in freedesktop.org some day).

If we need to maintain the contract that Neil explained above, then we may do the following:

if text/plain:
    QI to nsISupportsCString
    return single-byte string if available
elif text/unicode:
    QI to nsISupportsString
    return doublebyte string if available
elif QIs to nsISupportsCString
    return single-byte string
elif QIs to nsISupportsString
    return double-byte string

Sounds good?
Sounds good to me. How about you Neil?
ping!
It looks OK to me too, as long as the callers can deal with it.
Comment on attachment 345702 [details] [diff] [review]
don't fail to return a string if the string is nsISupportsCString but the flavor is not text/plain

Minusing because I think we need to do what comment #7 says
Attachment #345702 - Flags: review?(roc) → review-

Bulk-downgrade of unassigned, >=5 years untouched DOM/Storage bugs' priority.

If you have reason to believe this is wrong (especially for the severity), please write a comment and ni :jstutte.

Severity: normal → S4
Priority: -- → P5
You need to log in before you can comment on or make changes to this bug.