Bug 34030 – ClipboardMac::setData() should be case-insensitive for the format parameter

• Component: Platform
• Status: UNCONFIRMED

[Daniel Cheng]

A simple test case has been attached.

[Daniel Cheng]

Created attachment 47259 [details]
Test case

Really attach the attachment this time.

[Daniel Cheng]

Created attachment 47261 [details]
Patch

[Daniel Bates]

This bug is related to bug #31003.

[Daniel Cheng]

Created attachment 47263 [details]
IE test case

The spec has this to say:
Formats are generally given by MIME types, with some values special-cased for legacy reasons. For the purposes of this API, however, the format strings are opaque, case-sensitive, strings, and the empty string is a valid format string.

According to RFC 2616, "The type, subtype, and parameter attribute names are case-insensitive[ in a MIME type]."

If formats are typically MIME types, I think it would make sense to ignore case.
IE also ignores case, as the attached IE test case shows.

Currently, Mac and Qt both respect case. It seems only the Windows implementation does not.

[Darin Adler]

Comment on attachment 47261 [details]
Patch

The right way to be case insensitive is to use equalIgnoringCase, not to call lower.

[Daniel Cheng]

(In reply to comment #5)
> (From update of attachment 47261 [details])
> The right way to be case insensitive is to use equalIgnoringCase, not to call
> lower.

I would do that, but there's more than just string comparisons. For example:
>     if (qType == "text/plain" || qType.startsWith("text/plain;"))

I've also asked WHAT for clarification. I think there's a case to be made for ignoring case in format strings, but I'm going to hold the patch until there's a clear answer here.

[Daniel Cheng]

Created attachment 48090 [details]
Patch

[Daniel Cheng]

The HTML5 draft has changed on this point:
http://www.whatwg.org/specs/web-apps/current-work/multipage/dnd.html#datatransfer

"DataTransfer objects can hold pieces of data, each associated with a unique format. Formats are generally given by MIME types, with some values special-cased for legacy reasons. However, the API does not enforce this; non-MIME-type values can be added as well. All formats are identified by strings that are converted to ASCII lowercase by the API."

[Darin Adler]

Comment on attachment 48090 [details]
Patch

Change seems OK.

The preferred way to compare and ignore case differences is to use the equalIgnoringCase function and the CaseFoldingHash hash function. I'd prefer to see it working that way.

The use of "q" in "qType" is an ancient relic from when WebCore/platform didn’t exist and the code was using QString. So it’s strange to introduce it in new code.

[Daniel Cheng]

(In reply to comment #9)
> (From update of attachment 48090 [details])
> Change seems OK.
> 
> The preferred way to compare and ignore case differences is to use the
> equalIgnoringCase function and the CaseFoldingHash hash function. I'd prefer to
> see it working that way.

The way I interpret the spec, the UA ought to lower case all formats. That being said, I think there are problems with either approach.

If we only use equalIgnoringCase and CaseFoldingHash, it becomes much harder to deal with the system pasteboard. For example, in qt, removing a format from the clipboard is done with this snippet:
  m_writableData->removeFormat(qType);
To implement that, a UA would need to enumerate all types in the clipboard and then compare each one in a case-insensitive manner. In addition, what happens when multiple formats match?

However, if we convert everything to lowercase, then there are native pasteboard/drag and drop types the interface will never be able to retrieve.

Given the problems with both approaches, maybe the spec should actually say:
Only "text" and "url" should be treated in a case-insensitive manner; all other format names must be handled in a case-sensitive manner. What do you think?

> 
> The use of "q" in "qType" is an ancient relic from when WebCore/platform didn’t
> exist and the code was using QString. So it’s strange to introduce it in new
> code.

Interesting. I didn't know that; I took the naming from WebCore/platform/win. I'll change it with next update to this patch.

[David Levin]

Comment on attachment 48090 [details]
Patch

r- based on comments already in the bug.