1306645 - Can't paste image in clipboard with both CF_DIB and CF_HDROP format to Firefox

Status: VERIFIED FIXED

(Core :: Widget: Win32, defect)

Description

[fireattack [:fireattack]]

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/54.0.2840.41 Safari/537.36

Steps to reproduce:

1. Download XnView MP: http://www.xnview.com/en/xnviewmp/#downloads
2. Download Clipboard Format Spy: http://delphidabbler.com/software/cfs/download
3. Open Clipboard Format Spy
4. Use XnView MP to open an image (there are other software that has the same behavior, such as ACDSee; but it's not free)
5. Copy the image (Ctrl+C)
6. Check the information in CFS, it should be something like http://i.imgur.com/mcNjHRB.png
5. Go http://imgur.com/ and try to paste there


Actual results:

User should be able to paste image there and upload


Expected results:

User can't. If the clipboard only has CF_DIB, it works, though.

It works in Chrome, FWIW.

Comment 1

[ovidiu boca[:Ovidiu]]

Hi Benjamin,

I tested this issue on Windows 7 x64 with FF Nightly 52.0a1(2016-10-09) and I can't paste the image into http://imgur.com/. I also tested with Chrome and I couldn't upload the picture. 

Please create a new profile, you have the steps here:https://support.mozilla.org/en-US/kb/profile-manager-create-and-remove-firefox-profiles?redirectlocale=en-US&redirectslug=Managing-profiles#w_starting-the-profile-manager

Please test if the issue can be reproduced in the safe mode of Firefox: https://support.mozilla.org/en-US/kb/troubleshoot-firefox-issues-using-safe-mode

Comment 2

[fireattack [:fireattack]]

I didn't get what info you want from me. You already reproduced the issue, isn't it?

Comment 3

[fireattack [:fireattack]]

Anyway, I made a video to show the problem, if the STR above is not clear:

https://youtu.be/Xx0GaZG1IHI

I also showed it works fine in Chrome, although I don't know why you can't make it work there.

Comment 4

[ovidiu boca[:Ovidiu]]

Hi Benjamin,

I retested on Windows 10 x64 and I was able to reproduce this. 

This is a regression from bug Bug 1278939 - Store nsIFile entries as nsIFile, but continue to produce dom::File objects from relevant APIs, r=enndeakin

Here is the pushlog: https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a4c8c1c3ddfb6713833da131e7d7943815f8bc90&tochange=516c82acb163c7f75048b46118d5414db5cec558

Michael please take a look at this bug. Thanks

Comment 5

[Nika Layzell [:nika] (ni? for response)]

This appears to be the same issue as bug 1308007. Marking as duplicate.

Comment 6

[fireattack [:fireattack]]

Not the same bug. This one is about when BOTH CF_DIB and CF_HDROP exist in clipboard, you're unable to paste the image. But it works in Chrome.

That one is about only when CF_HDROP exists in your clipboard (Windows Photo Viewer behavior), you can't paste. That one IMO, is working as intended. CF_HDROP is just a handle to the file (not the image), why it should be pasted? 

Anyway, not the same bug.

Comment 7

[Nika Layzell [:nika] (ni? for response)]

(In reply to Benjamin Peng from comment #6)
> Not the same bug. This one is about when BOTH CF_DIB and CF_HDROP exist in
> clipboard, you're unable to paste the image. But it works in Chrome.
> 
> That one is about only when CF_HDROP exists in your clipboard (Windows Photo
> Viewer behavior), you can't paste. That one IMO, is working as intended.
> CF_HDROP is just a handle to the file (not the image), why it should be
> pasted? 
> 
> Anyway, not the same bug.

The issue is very related. The problem which is occurring here is that we prefer file objects when selecting which types to provide to the webpage from the OS provided clipboard, hiding the other types from the clipboard. Because we are provided a file, we thus choose that one. Unfortunately, we have screwed up our file logic and don't correctly pass the file to the child process. 

I believe that chrome simply doesn't support pasting files, which means that they ignore that type and select the CF_DIB type. 

This bug specifically, but not the general case could potentially also be fixed by making images be prioritized in this specific circumstance, or disabling the pasting of files in e10s, meaning that we never consider the option (because it is broken). mrbkap might have more opinions.

Comment 8

[fireattack [:fireattack]]

Ah, that makes sense. 

Feel free to change it back to duplicate once mrbkap put his opinions.

Comment 9

[Nika Layzell [:nika] (ni? for response)]

Attachment: Don't add application/x-moz-file entries from the clipboard to dataTransfer with e10s

application/x-moz-file entries on the clipboard are not able to be
successfully transmitted across synchronous IPC due to bug 1308007, thus
they appear as useless null files.

This patch hides those useless null files, and thus unhides image data
which may have been on the clipboard but was previously hidden by the
presence of an application/x-moz-file file

MozReview-Commit-ID: 4RmQe9kDZLe

Comment 10

[Neil Deakin]

For a workaround, why does this need to be Windows only?

Comment 11

[Nika Layzell [:nika] (ni? for response)]

Attachment: Don't add application/x-moz-file entries from the clipboard to dataTransfer with e10s

Updated

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3201c03e0c16
Don't add application/x-moz-file entries from the clipboard to dataTransfer with e10s, r=enndeakin

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3201c03e0c16

Comment 14

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8802957 [details] [diff] [review]
Don't add application/x-moz-file entries from the clipboard to dataTransfer with e10s

Approval Request Comment
[Feature/regressing bug #]: e10s
[User impact if declined]: DataTransfer will appear to contain a file, but when getting the value it will be null. In addition, this may cause images to be hidden from the clipboard when pasting both file and image data.
[Describe test coverage new/current, TreeHerder]: DataTransfer tends to have poor test coverage.
[Risks and why]: This patch is fairly low risk, it hides application/x-moz-file from clipboard datatransfer in e10s, which was a file type which has previously been broken.
[String/UUID change made/needed]: None.

Comment 15

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Comment on attachment 8802957 [details] [diff] [review]
Don't add application/x-moz-file entries from the clipboard to dataTransfer with e10s

This seems like a recent regression in 50, let's uplift.

Comment 16

[Ritu Kothari (:ritu) (Inactive, please n-i to RyanVM, jcristau, or pascal)]

Hello fireattack, could you please verify this issue is fixed as expected on a latest Nightly build? Thanks!

Comment 17

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-aurora/rev/f023ef728d1a

Comment 18

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/releases/mozilla-beta/rev/46279a9e334d

Comment 19

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

This appears to have caused a failure on beta: https://treeherder.mozilla.org/logviewer.html#?job_id=1851730&repo=mozilla-beta

Backed out in https://hg.mozilla.org/releases/mozilla-beta/rev/4131f9c0b0c7

Comment 20

[fireattack [:fireattack]]

I can confirm it's fixed on Nightly. 

However I saw some weird rendering on Imgur.com after uploading.. I assume it's not relevant to this patch right? Check https://www.youtube.com/watch?v=HCmoHJ9qgE8

Comment 21

[Nika Layzell [:nika] (ni? for response)]

Attachment: Part 2: Disable browser_clipboard_pastefile with e10s enabled

This test is testing things which don't make sense with this fix applied, which is why it failed. I think it should be OK to disable it here.

In M-C this test is actually disabled completely:
http://searchfox.org/mozilla-central/rev/e3e8571c5378ac92663d4f583ccc4ad0a3019716/browser/base/content/test/general/browser.ini#288

Comment 22

[Nika Layzell [:nika] (ni? for response)]

Comment on attachment 8804320 [details] [diff] [review]
Part 2: Disable browser_clipboard_pastefile with e10s enabled

Approval Request Comment

This fixes the test failure which caused the other patch on this bug to be backed out of beta.

Comment 23

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 8804320 [details] [diff] [review]
Part 2: Disable browser_clipboard_pastefile with e10s enabled

Test-only changes don't need approval. I'll just fold this into the original patch and re-land.

Comment 24

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-beta/rev/a258a83d217d

Comment 25

[Ciprian Georgiu, Desktop QA]

I've managed to reproduce this issue with a Nightly build from 2016-09-30 using STR from comment 0.

This is verified fixed on 50.0b11 (20161027110534) and latest Aurora 51.0a2 (2016-10-27) under Windows 10 x64. Marking here accordingly.