Closed Bug 1305163 Opened 3 years ago Closed 3 years ago

FF50b1: Double Images Created when using CTRL-V at Imgur.com

Categories

(Core :: DOM: Events, defect, P2)

50 Branch
x86_64
Windows 7
defect

Tracking

()

RESOLVED FIXED
Tracking Status
firefox49 --- unaffected
firefox-esr45 --- unaffected
firefox50 --- verified
firefox51 --- verified
firefox52 --- unaffected

People

(Reporter: bugzilla, Assigned: stone)

References

Details

(Keywords: regression)

Attachments

(3 files, 3 obsolete files)

User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:50.0) Gecko/20100101 Firefox/50.0
Build ID: 20160920155715

Steps to reproduce:

Go to imgur.com (while logged in) and press CTRL-V to upload an image via the clipboard. 


Actual results:

When pressing CTRL-V, two new images were created out of what was in the clipboard: http://i.imgur.com/6gZGQUQ.png (the two on the top-right). What's strange is that one is a .PNG http://i.imgur.com/7YkBpLw.png and one is a .JPG http://i.imgur.com/ulUdiXj.jpg I then have to delete the extra one.


Expected results:

Only one new image should have be been created I would think, as happens in previous versions of Firefox. They only make the single first .PNG image.  So, something is different with FF50 and the clipboard and what it does with CTRL-V, at least on Imgur.
I'm not able to reproduce it with FF50 or 52. Only one instance of the image is uploaded.

Are you able to reproduce the issue with a clean profile?
https://support.mozilla.org/fr/kb/utiliser-gestionnaire-profils-creer-supprimer-profils
Flags: needinfo?(bugzilla)
Does the same thing with a clean profile. When I press CTRL-V is actually says "2 images uploading".  Tried both the New Post page and the Images page. http://i.imgur.com/Un6dsq5.png
Flags: needinfo?(bugzilla)
OS: Unspecified → Windows 7
Hardware: Unspecified → x86_64
I did another test where I set all the plugins to never activate and disabled the Roboform Toolbar (only extension listed). When I press CTRL-V it uploads 2 images: Snapshot of this showing 2 images uploading: http://i.imgur.com/X7ya3Li.png
I dunno which step I miss to be not able to reproduce the bug.

Did you enable e10s in FF50?
No, it was disabled in both normal and clean profiles: Multiprocess Windows 	0/1 (Disabled)

I then made a new "betaclean" profile so it didn't share with my normal Firefox. This time Multiprocess was enabled but it still pasted 2 images with CTRL-V.
Oh before, was running the same "clean" profile in both FF49 and 50 and it pasted 1 image with 49 and 2 images with 50.
Hello,

MarkRH is right, i sign up with http://imgur.com and i  reproduced the same  bug again. It does uploads 2 images with ctrl + v. i tested it with firefox 49. would someone point me to the right direction of how to fix this bug?

thanks
(In reply to Onek Jude from comment #7)
> Hello,
> 
> MarkRH is right, i sign up with http://imgur.com and i  reproduced the same 
> bug again. It does uploads 2 images with ctrl + v. i tested it with firefox
> 49. would someone point me to the right direction of how to fix this bug?
> 
> thanks

Note: I do not have the problem with Firefox 49 like Onek does. I see the problem in Firefox 50 beta 1, with and without clean profiles.
Are you connetced to Imgur through a social account like Facebook?
I don't have Imgur directly connected to any social media and am not using one of those to log into it with.
Same happens with 50beta2.
I am able to reproduce this issue with FF 50 and 51 on Windows 7 and Windows 10, two images are getting uploaded.
On FF 52 ctr + v is not working at all/ no image is getting uploaded.
On Ff 49 ctr + v works fine and only one image gets uploaded.
Status: UNCONFIRMED → NEW
Component: Untriaged → Keyboard: Navigation
Ever confirmed: true
Product: Firefox → Core
(In reply to Kanchan Kumari QA from comment #12)
> I am able to reproduce this issue with FF 50 and 51 on Windows 7 and Windows
> 10, two images are getting uploaded.
> On FF 52 ctr + v is not working at all/ no image is getting uploaded.
> On Ff 49 ctr + v works fine and only one image gets uploaded.

Hi Kanchan, would you be able to get the regression window? Thanks!
Flags: needinfo?(kkumari)
Regression range: 
Last good revision: b2409b36cd07bba045bf38dd131166c36ea3da4a
First bad revision: 3d0aef4586bdbd81591811ec627c85161fc11065

Pushlog:
https://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=b2409b36cd07bba045bf38dd131166c36ea3da4a&tochange=3d0aef4586bdbd81591811ec627c85161fc11065
Flags: needinfo?(kkumari)
Blocks: 906420
Component: Keyboard: Navigation → DOM: Events
Same thing happens in https://web.telegram.org/ or other places that read the images from the clipboard like http://www.martinezdelizarrondo.com/ckplugins/simpleuploads.demo4/

They don't expect two versions of the same image on the clipboard.
I think that this should be fixed in 51 because of bug 1290688

Perhaps we want to uplift?
(In reply to Michael Layzell [:mystor] from comment #16)
> I think that this should be fixed in 51 because of bug 1290688
> 
> Perhaps we want to uplift?

(Namely, we would only have to uplift part 1 which is very low risk)
Either uplift the fix or backout the regressing patch.
(In reply to Olli Pettay [:smaug] from comment #18)
> Either uplift the fix or backout the regressing patch.

The regressing patch is pretty big. It will be much easier to uplift the 2-line fix.
(In reply to Michael Layzell [:mystor] from comment #16)
> I think that this should be fixed in 51 because of bug 1290688
> 

See comment#12, Aurora 51 also affected. Nightly 52 as well(worse).
This is odd. It seems to work for me on linux. I went to google search results, copied an image, and pasted it into the imgur upload gui. It only uploads a single copy of the image on nightly.
Using "Copy" from an image, only one image is pasted. 
But taking a screenshot, both a png and jpg are pasted
See Also: → 1307999, 1308007
I filed bug for the problem comment #15.
bug 1307999, bug 1308007
FYI, If I copy part of an image using Irfanview, I get two images uploaded. If I right-click an image on a Google image search and select copy image, I get two images when doing CTRL-V. Using 50 beta 4.
(In reply to Michael Layzell [:mystor] from comment #21)
> This is odd. It seems to work for me on linux. I went to google search
> results, copied an image, and pasted it into the imgur upload gui. It only
> uploads a single copy of the image on nightly.

This bug was reported on Windows, and likely platform-specific depending on clipboard formats we support, now that we push formats up in a loop:

https://bugzilla.mozilla.org/attachment.cgi?id=8766012&action=diff#a/dom/events/DataTransfer.cpp_sec8
Flags: needinfo?(michael)
I'm making this bug be about the duplicate image issue. I believe that the no image issue is bug 1308007.

I believe that this issue should be fixable through some uplifts of bug 1290688 combined with bug 1309645.
Flags: needinfo?(michael)
Priority: -- → P2
Stone will be helping uplifting this to aurora and beta.
Please let us know if there are more details we need to care. Thanks!
Assignee: nobody → sshih
This should make it such that we only expose a single type of image through the DataTransfer. This patch should be able to apply on BETA.
Attachment #8802311 - Flags: review?(enndeakin)
And the AURORA patch.
Attachment #8802312 - Flags: review?(enndeakin)
Do these patches and the original one inadvertently remove rtf?
Rebased patch. Tested on windows10 and passed.
Rebased patch. Tested on windows10 and passed.
Comment on attachment 8802800 [details] [diff] [review]
BETA: Don't expose multiple image types through DataTransfer (rebased)

Let's file a bug to fix the missing rtf separately (and probably combine the two lists).
Attachment #8802800 - Flags: review+
Attachment #8802801 - Flags: review+
Attachment #8802311 - Flags: review?(enndeakin)
Attachment #8802312 - Flags: review?(enndeakin)
Hi Michael,
Could you help to create the uplift requests if the tried results are ok for you? Because I have no idea about how to fill the required information. Thanks.
Flags: needinfo?(michael)
Comment on attachment 8802800 [details] [diff] [review]
BETA: Don't expose multiple image types through DataTransfer (rebased)

Approval Request Comment
[Feature/regressing bug #]: bug 906420
[User impact if declined]: Attempts to copy/paste images will cause multiple images to be present in the paste.
[Describe test coverage new/current, TreeHerder]: DataTransfer generally has poor test coverage.
[Risks and why]: This patch generally doesn't change high risk parts of the codebase, instead simply causing certain image mime types to not be considered for inclusion into the DataTransfer. I think this is fairly low risk.
[String/UUID change made/needed]: None
Flags: needinfo?(michael)
Attachment #8802800 - Flags: approval-mozilla-beta?
Comment on attachment 8802801 [details] [diff] [review]
AURORA: Don't expose multiple image types through DataTransfer (rebased)

Approval Request Comment
[Feature/regressing bug #]: bug 906420
[User impact if declined]: Attempts to copy/paste images will cause multiple images to be present in the paste.
[Describe test coverage new/current, TreeHerder]: DataTransfer generally has poor test coverage.
[Risks and why]: This patch generally doesn't change high risk parts of the codebase, instead simply causing certain image mime types to not be considered for inclusion into the DataTransfer. I think this is fairly low risk.
[String/UUID change made/needed]: None
Attachment #8802801 - Flags: approval-mozilla-aurora?
Comment on attachment 8802800 [details] [diff] [review]
BETA: Don't expose multiple image types through DataTransfer (rebased)

Recent visible regression in a core scenario, Beta50+.

Hi Andrei, could you please do some exploratory testing around the STRs? This fix may need verification on our side to be sure we haven't caused any unexpected regressions.
Flags: needinfo?(andrei.vaida)
Attachment #8802800 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Attachment #8802801 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
still has problems uplifting to aurora:

renamed 1305163 -> bug-1305163-aurora-v3.patch
applying bug-1305163-aurora-v3.patch
patching file dom/events/DataTransfer.cpp
Hunk #4 FAILED at 1413
1 out of 4 hunks FAILED -- saving rejects to file dom/events/DataTransfer.cpp.rej
patch failed, unable to continue (try -v)
patch failed, rejects left in working directory
errors during apply, please fix and qrefresh bug-1305163-aurora-v3.patch
Flags: needinfo?(sshih)
Attachment #8802801 - Attachment is obsolete: true
Flags: needinfo?(sshih)
Attachment #8804526 - Flags: review+
Attachment #8802311 - Attachment is obsolete: true
Attachment #8802312 - Attachment is obsolete: true
Attached image imgur-bad-rendering.png
When I re-tested the patch, I found the rendering results seems NG. I don't know if it's a known issue so still update here. The screen shot "imgur-bad-rendering.png" is uploaded.
(In reply to Ritu Kothari (:ritu) from comment #38)
> Comment on attachment 8802800 [details] [diff] [review]
> BETA: Don't expose multiple image types through DataTransfer (rebased)
> 
> Recent visible regression in a core scenario, Beta50+.
> 
> Hi Andrei, could you please do some exploratory testing around the STRs?
> This fix may need verification on our side to be sure we haven't caused any
> unexpected regressions.

We'll make sure this is included in our 50.0b11 sign off. Keeping the ni? until we're done.
Flags: qe-verify+
I was able to reproduce the issue described in comment 0 *but* only if I pasted the image stored in clipboard after I took a screenshot or copied from websites directly. I was not able to copy/paste images from local drive (using 49 I could).

I verified that using Firefox 50 beta 11 on Windows 10 64bit, only one image will be pasted in imgur but again, from screenshot or copied from websites directly, images from local drive don't work. Not sure if this is something intended (it should not be IMHO). 
Ming-Chou Shih maybe you can clear this up?
Flags: needinfo?(andrei.vaida) → needinfo?(sshih)
Looks like it's not related to this patch. I rollback the patch and still unable to paste images from local drive.
Flags: needinfo?(sshih)
(In reply to Ming-Chou Shih [:stone] from comment #46)
> Looks like it's not related to this patch. I rollback the patch and still
> unable to paste images from local drive.

Well, then I'm gonna mark as verified on 50 and 51 (I also tested on latest Developer Edition) since the double items issue is fixed (bug description) and for the other issue with local files I think bug 1308007 reflects this behavior, the only difference is that it does not reproduce only on e10s.

Also on latest Developer Edition I was able to see the graphic issue mentioned in comment 41.
(In reply to Bogdan Maris, QA [:bogdan_maris] from comment #45)
> I was able to reproduce the issue described in comment 0 *but* only if I
> pasted the image stored in clipboard after I took a screenshot or copied
> from websites directly. I was not able to copy/paste images from local drive
> (using 49 I could).
> 
> I verified that using Firefox 50 beta 11 on Windows 10 64bit, only one image
> will be pasted in imgur but again, from screenshot or copied from websites
> directly, images from local drive don't work. Not sure if this is something
> intended (it should not be IMHO). 

This is the expected behavior because of bug 1308007. We're hoping to clear it up at some point, but for now copy and paste of files doesn't work with e10s enabled. You were probablty able to make it work on 49 because you don't have e10s enabled.
Just tried 50beta11 and I only get one image when CTRL-V at imgur.com now. This was from doing a ALT-PrintScreen and copying part of an image from Irfanview.  So, this seems to have solved the issue in my case. Thanks for everyone's work.
According to the comment49 and tracking flags, time to close this as Resolved/Fixed.
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.