Bug 1342057 - Part 1: Use correct MIME type for files as DataTransferItem.type, r=baku
authorMichael Layzell <michael@thelayzells.com>
Fri, 24 Feb 2017 16:06:35 -0500
changeset 381687 742fc1cf3d2895bf109cfed11a4df276d5df3bed
parent 381686 79c244d2866e5220ec809da0dbc258de76635688
child 381688 5891f51354b2aee761210589d35b2bd92470ba54
push idunknown
push userunknown
push dateunknown
reviewersbaku
bugs1342057
milestone55.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1342057 - Part 1: Use correct MIME type for files as DataTransferItem.type, r=baku MozReview-Commit-ID: 4hrZ3YoGTJj
dom/events/DataTransfer.cpp
dom/events/DataTransferItem.cpp
dom/events/DataTransferItem.h
dom/events/DataTransferItemList.cpp
dom/events/test/test_DataTransferItemList.html
--- a/dom/events/DataTransfer.cpp
+++ b/dom/events/DataTransfer.cpp
@@ -327,18 +327,21 @@ DataTransfer::GetTypes(nsTArray<nsString
   for (uint32_t i = 0; i < items->Length(); i++) {
     DataTransferItem* item = items->ElementAt(i);
     MOZ_ASSERT(item);
 
     if (item->ChromeOnly() && aCallerType != CallerType::System) {
       continue;
     }
 
+    // NOTE: The reason why we get the internal type here is because we want
+    // kFileMime to appear in the types list for backwards compatibility
+    // reasons.
     nsAutoString type;
-    item->GetType(type);
+    item->GetInternalType(type);
     if (item->Kind() == DataTransferItem::KIND_STRING || type.EqualsASCII(kFileMime)) {
       // If the entry has kind KIND_STRING, we want to add it to the list.
       aTypes.AppendElement(type);
     }
   }
 
   for (uint32_t i = 0; i < mItems->Length(); ++i) {
     bool found = false;
@@ -516,18 +519,21 @@ DataTransfer::MozTypesAt(uint32_t aIndex
     const nsTArray<RefPtr<DataTransferItem>>& items = *mItems->MozItemsAt(aIndex);
 
     bool addFile = false;
     for (uint32_t i = 0; i < items.Length(); i++) {
       if (items[i]->ChromeOnly() && aCallerType != CallerType::System) {
         continue;
       }
 
+      // NOTE: The reason why we get the internal type here is because we want
+      // kFileMime to appear in the types list for backwards compatibility
+      // reasons.
       nsAutoString type;
-      items[i]->GetType(type);
+      items[i]->GetInternalType(type);
       if (NS_WARN_IF(!types->Add(type))) {
         aRv.Throw(NS_ERROR_FAILURE);
         return nullptr;
       }
 
       if (items[i]->Kind() == DataTransferItem::KIND_FILE) {
         addFile = true;
       }
@@ -976,17 +982,17 @@ DataTransfer::GetTransferable(uint32_t a
     for (uint32_t f = 0; f < count; f++) {
       RefPtr<DataTransferItem> formatitem = item[f];
       nsCOMPtr<nsIVariant> variant = formatitem->DataNoSecurityCheck();
       if (!variant) { // skip empty items
         continue;
       }
 
       nsAutoString type;
-      formatitem->GetType(type);
+      formatitem->GetInternalType(type);
 
       // If the data is of one of the well-known formats, use it directly.
       bool isCustomFormat = true;
       for (uint32_t f = 0; f < ArrayLength(knownFormats); f++) {
         if (type.EqualsASCII(knownFormats[f])) {
           isCustomFormat = false;
           break;
         }
--- a/dom/events/DataTransferItem.cpp
+++ b/dom/events/DataTransferItem.cpp
@@ -225,16 +225,47 @@ DataTransferItem::FillInExternalData()
   SetData(variant);
 
   if (oldKind != Kind()) {
     NS_WARNING("Clipboard data provided by the OS does not match predicted kind");
     mDataTransfer->TypesListMayHaveChanged();
   }
 }
 
+void
+DataTransferItem::GetType(nsAString& aType)
+{
+  // If we don't have a File, we can just put whatever our recorded internal
+  // type is.
+  if (Kind() != KIND_FILE) {
+    aType = mType;
+    return;
+  }
+
+  // If we do have a File, then we need to look at our File object to discover
+  // what its mime type is. We can use the System Principal here, as this
+  // information should be avaliable even if the data is currently inaccessible
+  // (for example during a dragover).
+  //
+  // XXX: This seems inefficient, as it seems like we should be able to get this
+  // data without getting the entire File object, which may require talking to
+  // the OS.
+  ErrorResult rv;
+  RefPtr<File> file = GetAsFile(*nsContentUtils::GetSystemPrincipal(), rv);
+  MOZ_ASSERT(!rv.Failed(), "Failed to get file data with system principal");
+
+  // If we don't actually have a file, fall back to returning the internal type.
+  if (NS_WARN_IF(!file)) {
+    aType = mType;
+    return;
+  }
+
+  file->GetType(aType);
+}
+
 already_AddRefed<File>
 DataTransferItem::GetAsFile(nsIPrincipal& aSubjectPrincipal,
                             ErrorResult& aRv)
 {
   // This is done even if we have an mCachedFile, as it performs the necessary
   // permissions checks to ensure that we are allowed to access this type.
   nsCOMPtr<nsIVariant> data = Data(&aSubjectPrincipal, aRv);
   if (NS_WARN_IF(!data || aRv.Failed())) {
--- a/dom/events/DataTransferItem.h
+++ b/dom/events/DataTransferItem.h
@@ -64,21 +64,23 @@ public:
       aKind = NS_LITERAL_STRING("string");
       return;
     default:
       aKind = NS_LITERAL_STRING("other");
       return;
     }
   }
 
-  void GetType(nsAString& aType) const
+  void GetInternalType(nsAString& aType) const
   {
     aType = mType;
   }
 
+  void GetType(nsAString& aType);
+
   eKind Kind() const
   {
     return mKind;
   }
 
   already_AddRefed<File>
   GetAsFile(nsIPrincipal& aSubjectPrincipal, ErrorResult& aRv);
 
--- a/dom/events/DataTransferItemList.cpp
+++ b/dom/events/DataTransferItemList.cpp
@@ -289,18 +289,19 @@ DataTransferItemList::MozRemoveByTypeAt(
 
     // items is no longer a valid reference, as removing the last element from
     // it via ClearDataHelper invalidated it. so we can't MOZ_ASSERT that the
     // length is now 0.
     return;
   }
 
   for (uint32_t i = 0; i < count; ++i) {
+    // NOTE: As this is a moz-prefixed API, it works based on internal types.
     nsAutoString type;
-    items[i]->GetType(type);
+    items[i]->GetInternalType(type);
     if (type == aType) {
       ClearDataHelper(items[i], -1, i, aSubjectPrincipal, aRv);
       return;
     }
   }
 }
 
 DataTransferItem*
@@ -308,18 +309,19 @@ DataTransferItemList::MozItemByTypeAt(co
 {
   if (NS_WARN_IF(aIndex >= mIndexedItems.Length())) {
     return nullptr;
   }
 
   uint32_t count = mIndexedItems[aIndex].Length();
   for (uint32_t i = 0; i < count; i++) {
     RefPtr<DataTransferItem> item = mIndexedItems[aIndex][i];
+    // NOTE: As this is a moz-prefixed API it works on internal types
     nsString type;
-    item->GetType(type);
+    item->GetInternalType(type);
     if (type.Equals(aType)) {
       return item;
     }
   }
 
   return nullptr;
 }
 
@@ -333,17 +335,17 @@ DataTransferItemList::SetDataWithPrincip
                                            ErrorResult& aRv)
 {
   if (aIndex < mIndexedItems.Length()) {
     nsTArray<RefPtr<DataTransferItem>>& items = mIndexedItems[aIndex];
     uint32_t count = items.Length();
     for (uint32_t i = 0; i < count; i++) {
       RefPtr<DataTransferItem> item = items[i];
       nsString type;
-      item->GetType(type);
+      item->GetInternalType(type);
       if (type.Equals(aType)) {
         if (NS_WARN_IF(aInsertOnly)) {
           aRv.Throw(NS_ERROR_DOM_NOT_SUPPORTED_ERR);
           return nullptr;
         }
 
         // don't allow replacing data that has a stronger principal
         bool subsumes;
--- a/dom/events/test/test_DataTransferItemList.html
+++ b/dom/events/test/test_DataTransferItemList.html
@@ -93,17 +93,17 @@
          "Adding a non-file entry to a non-zero index should not add an item to the items list");
 
       var file3 = new File(['<a id="e"><b id="f">heya!</b></a>'], "yetanotherfile.html",
                            {type: "text/html"});
       e.dataTransfer.mozSetDataAt("random/string", file3, 3);
       is(oldLength + 1, dtList.length,
          "Replacing the entry with a file should add it to the list!");
       is(dtList[oldLength].getAsFile(), file3, "It should be stored in the last index as a file");
-      is(dtList[oldLength].type, "random/string", "It should have the correct type");
+      is(dtList[oldLength].type, "text/html", "It should have the correct type");
       is(dtList[oldLength].kind, "file", "It should have the correct kind");
 
       todo(files.length == 3, "This test has chrome privileges, so the FileList objects aren't updated live");
       files = e.dataTransfer.files;
       is(files[files.length - 1], file3, "It should also be in the files list");
 
       oldLength = dtList.length;
       var nonstring = {};