Bug 1342057 - Part 1: Use correct MIME type for files as DataTransferItem.type. r=baku, a=lizzard
authorMichael Layzell <michael@thelayzells.com>
Fri, 24 Feb 2017 16:06:35 -0500
changeset 379015 ccc12c0fbf9fc09ee395015a4b67cb57e47e41c3
parent 379014 37fb7f53b4ac506eca51d0ce4724de71b26d3ccc
child 379016 fc832b9c1e62f84fbadd57954fec7e9cb49688d0
push id1419
push userjlund@mozilla.com
push dateMon, 10 Apr 2017 20:44:07 +0000
treeherdermozilla-release@5e6801b73ef6 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersbaku, lizzard
bugs1342057
milestone53.0
Bug 1342057 - Part 1: Use correct MIME type for files as DataTransferItem.type. r=baku, a=lizzard 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
@@ -324,18 +324,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;
@@ -513,18 +516,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;
       }
@@ -973,17 +979,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 = {};