Verified Commit 8bb484e1 authored by David Parks's avatar David Parks Committed by ma1
Browse files

Bug 2057113 - Part 2: Don't force a synchronous clipboard flush for file... 

Bug 2057113 - Part 2: Don't force a synchronous clipboard flush for file promises.  a=diannaS DONTBUILD

SetNativeClipboardData flushes clipboard writes when text/plain is on the
transferable on Windows 11 22H2+. Flushing was added to clipboard write in
bug 1774285, because delayed rendering makes Windows Suggested Actions walk
the a11y tree, and immediate rendering of data to the clipboard was the lesser
evil.

Rendering a file promise maps to CF_HDROP, whose renderer is
nsDataObj::DropTempFile (which is unintuitively _not_ used for drag and drop,
only clipboard).  It opens a channel to the promised URL and runs a nested
event loop in CStream::WaitForCompletion until it completes, so forcing the
flush runs a network fetch in a nested event loop inside the clipboard commit.
The duration of the nest is unpredictable but can be as long as the device
maintains the connection (modulo any timeout limits).   This patch skips the
flush if there is a file (or no text) in the payload, avoiding the nested loop
altogether when this happens.

Without a pref flip (clipboard.imageAsFile.enabled = true), writing both
a file and text to clipboard should not be possible, so this case does not
occur.  This is because only images can be written to the clipboard as files.
See [the MDN page for mandatory types](https://www.w3.org/TR/clipboard-apis/#mandatory-data-types)
and the comment in ClipboardItem::Supports.

Original Revision: https://phabricator.services.mozilla.com/D326256

Differential Revision: https://phabricator.services.mozilla.com/D328682
parent 3f7b1f31

widget/windows/nsClipboard.cpp

+21 −4
Changes for widget/windows/nsClipboard.cpp: 21 added lines, 4 removed lines.
Original line number Diff line number Diff line
@@ -219,6 +219,9 @@ nsresult nsClipboard::SetupNativeDataObject( 
  nsTArray<nsCString> flavors;

  aTransferable->FlavorsTransferableCanExport(flavors);



  bool hasText = false;

  bool hasFilePromise = false;



  // Walk through flavors that contain data and register them

  // into the DataObj as supported flavors

  for (uint32_t i = 0; i < flavors.Length(); i++) {
@@ -235,6 +238,11 @@ nsresult nsClipboard::SetupNativeDataObject( 
    SET_FORMATETC(fe, format, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL);

    dObj->AddDataFlavor(flavorStr.get(), &fe);



    if (flavorStr.EqualsLiteral(kFilePromiseMime) ||

        flavorStr.EqualsLiteral(kFilePromiseURLMime)) {

      hasFilePromise = true;

    }



    // Do various things internal to the implementation, like map one

    // flavor to another or add additional flavors based on what's required

    // for the win32 impl.
@@ -244,9 +252,7 @@ nsresult nsClipboard::SetupNativeDataObject( 
      FORMATETC textFE;

      SET_FORMATETC(textFE, CF_TEXT, 0, DVASPECT_CONTENT, -1, TYMED_HGLOBAL);

      dObj->AddDataFlavor(kTextMime, &textFE);

      if (aMightNeedToFlush) {

        *aMightNeedToFlush = MightNeedToFlush::Yes;

      }

      hasText = true;

    } else if (flavorStr.EqualsLiteral(kHTMLMime)) {

      // if we find text/html, also advertise win32's html flavor (which we will

      // convert on our own in nsDataObj::GetText().
@@ -320,6 +326,18 @@ nsresult nsClipboard::SetupNativeDataObject( 
    }

  }



  if (aMightNeedToFlush) {

    // We flush in order to stop Windows Suggested Actions walking the a11y

    // tree, which it only does for text (bug 1774285).  Rendering a file

    // promise, however, fetches the promised URL under a nested event loop, so

    // a transferable carrying both would trade the tree walk for a main-thread

    // network fetch.  We choose the tree-walk penalty instead, in this case.

    // This means that bug 1774285 reappears for that combination.  However,

    // the combination would naturally be very rare (clipboard ops including

    *aMightNeedToFlush = hasText && !hasFilePromise ? MightNeedToFlush::Yes

                                                    : MightNeedToFlush::No;

  }



  if (!mozilla::StaticPrefs::

          clipboard_copyPrivateDataToClipboardCloudOrHistory()) {

    // Let Clipboard know that data is sensitive and must not be copied to
@@ -329,7 +347,6 @@ nsresult nsClipboard::SetupNativeDataObject( 
      nsresult rv =

          StoreValueInDataObject(dObj, TEXT("CanUploadToCloudClipboard"), 0);

      NS_ENSURE_SUCCESS(rv, rv);

      rv =

          StoreValueInDataObject(dObj, TEXT("CanIncludeInClipboardHistory"), 0);

      NS_ENSURE_SUCCESS(rv, rv);

      rv = StoreValueInDataObject(