[Webkit-unassigned] [Bug 28293] [Chromium] event.datatransfer.getdata("text/uri-list") treated the same as getdata("URL")
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Tue Sep 1 09:54:33 PDT 2009
https://bugs.webkit.org/show_bug.cgi?id=28293
David Levin <levin at chromium.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Attachment #38603|review? |review-
Flag| |
--- Comment #11 from David Levin <levin at chromium.org> 2009-09-01 09:54:33 PDT ---
(From update of attachment 38603)
Sorry for taking so long to get back to this. Just a few last things to
address.
> Index: WebCore/ChangeLog
> ===================================================================
> + This patch does not include a layout test, since Chromium does not yet support the
> + necessary interfaces.
> + (cf. LayoutTests/http/tests/security/clipboard/clipboard-file-access.html)
Just include it. :)
> Index: WebCore/platform/chromium/ClipboardChromium.cpp
> ===================================================================
> --- WebCore/platform/chromium/ClipboardChromium.cpp (revision 47748)
> +++ WebCore/platform/chromium/ClipboardChromium.cpp (working copy)
> @@ -52,17 +52,24 @@
> // We provide the IE clipboard types (URL and Text), and the clipboard types specified in the WHATWG Web Applications 1.0 draft
> // see http://www.whatwg.org/specs/web-apps/current-work/ Section 6.3.5.3
It isn't part of your patch but I looked at this to validate this addition and
the section reference is incorrect. Perhaps instead of referring to a
numerical section it could just give the spec url and a section title or
something less mutable?
>
> +enum ClipboardDataType { ClipboardDataTypeNone,
> + ClipboardDataTypeURL,
> + ClipboardDataTypeURIList,
> + ClipboardDataTypeText };
This is formatted in an odd way. I would expect something like this:
enum ClipboardDataType {
ClipboardDataTypeNone,
ClipboardDataTypeURL,
ClipboardDataTypeURIList,
ClipboardDataTypeText
};
> + if (cleanType == "url")
> + return ClipboardDataTypeURL;
> + if (cleanType == "text/uri-list")
> + return ClipboardDataTypeURIList;
> + // includes two special cases for IE compatibility
Format comments like sentences (start with capital, end with period).
// Includes two special cases for IE compatibility.
> + switch (clipboardTypeFromMIMEType(type)) {
> + case ClipboardDataTypeURL:
> m_dataObject->url = KURL();
> m_dataObject->urlTitle = "";
> + break;
> +
> + case ClipboardDataTypeURIList:
> + m_dataObject->filenames.clear();
> + break;
Why doesn't this fall through to ClipboardDataTypeURL since the handling of
this type does?
> +
> + case ClipboardDataTypeText:
> + m_dataObject->plainText = "";
> + break;
Add a
case ClipboardDataTypeNone:
break;
to allow for turning on warnings that all enum values are handled.
Also, I would change all "break;" to "return;" and add a ASSERT_NOT_REACHED();
at the end of this function.
> + case ClipboardDataTypeURL:
> // FIXME: Handle the cut/paste event. This requires adding a new IPC
> // message to get the URL from the clipboard directly.
> if (!m_dataObject->url.isEmpty()) {
> success = true;
> text = m_dataObject->url.string();
> }
> + break;
Add a
case ClipboardDataTypeNone:
break;
to allow for turning on warnings that all enum values are handled.
> }
> + switch (clipboardTypeFromMIMEType(type)) {
> + case ClipboardDataTypeURL:
> m_dataObject->url = KURL(data);
> return m_dataObject->url.isValid();
> + case ClipboardDataTypeURIList:
> + data.split(filenamesSeparator, m_dataObject->filenames);
> + return true;
> +
> + case ClipboardDataTypeText:
> m_dataObject->plainText = data;
> return true;
Add a
case ClipboardDataTypeNone:
return false;
to allow for turning on warnings that all enum values are handled.
Also, I would add a ASSERT_NOT_REACHED(); at the end of this function.
> Index: WebCore/platform/chromium/ClipboardChromiumWin.cpp
> ===================================================================
> +String ClipboardChromium::filenameAsURL(String filename)
> +{
> + if (filename.isEmpty())
> + return String();
> +
> + wchar_t buffer[INTERNET_MAX_URL_LENGTH];
> + DWORD length = sizeof(buffer)/sizeof(buffer[0]);
Add spaces around /
--
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.
More information about the webkit-unassigned
mailing list