Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ecau): drop images without jQuery #472

Merged
merged 1 commit into from
Jun 12, 2022
Merged

fix(ecau): drop images without jQuery #472

merged 1 commit into from
Jun 12, 2022

Conversation

ROpdebee
Copy link
Owner

@ROpdebee ROpdebee commented Jun 11, 2022

So apparently my previous experimentation didn't go far enough and it turns out we don't actually need jQuery to get MB to handle the drop event. The reason we were using jQuery was that we can't "officially" instantiate our own DropEvents with a FileList with our own files in the dataTransfer property (either because of read-only properties, private constructors, or some other security precaution). However, I never tried just defining the necessary properties using Object.defineProperty, which can circumvent the readonly status. Turns out, that just works. 🎉

Fixes #440.

Tested successfully on:

  • Firefox/Violentmonkey
  • Firefox/Violentmonkey BETA
  • Firefox/Tampermonkey
  • Firefox/Greasemonkey 4
  • Chromium/Violentmonkey
  • Chromium/Violentmonkey BETA
  • Chromium/Tampermonkey
  • Waterfox/Greasemonkey 3

@ROpdebee ROpdebee added bug Something isn't working mb_enhanced_cover_art_uploads labels Jun 11, 2022
@ROpdebee ROpdebee requested a review from kellnerd June 11, 2022 18:10
@codecov
Copy link

codecov bot commented Jun 11, 2022

Codecov Report

Merging #472 (792e4b9) into main (e3d776f) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##              main      #472   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           46        46           
  Lines         1008      1009    +1     
  Branches       168       168           
=========================================
+ Hits          1008      1009    +1     
Impacted Files Coverage Δ
src/mb_enhanced_cover_art_uploads/form.ts 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e3d776f...792e4b9. Read the comment docs.

@ROpdebee
Copy link
Owner Author

/deploy-preview please.

@github-actions
Copy link

github-actions bot commented Jun 11, 2022

This PR changes 1 built userscript(s):

See all changes

github-actions bot added a commit that referenced this pull request Jun 11, 2022
fix(ecau): drop images without jQuery (#472)
@ROpdebee ROpdebee force-pushed the ecau-new-form-input branch from 45085f1 to 41e6e35 Compare June 11, 2022 19:10
@ROpdebee
Copy link
Owner Author

ROpdebee commented Jun 11, 2022

/deploy-preview with Greasemonkey fixes

Tested successfully with all major browser/userscript engine combinations 🎉

Greasemonkey was being naughty again though, but that's also been fixed.

@ROpdebee
Copy link
Owner Author

/deploy-preview because the bot doesn't listen to edited comments 🙄

github-actions bot added a commit that referenced this pull request Jun 11, 2022
fix(ecau): drop images without jQuery (#472)
@ROpdebee ROpdebee force-pushed the ecau-new-form-input branch from 41e6e35 to 792e4b9 Compare June 12, 2022 09:29
Copy link
Collaborator

@kellnerd kellnerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎉

P.S. When you say that you've tested with VM, that includes VM Beta, right?

@ROpdebee
Copy link
Owner Author

Good point, I actually only tested with VM Beta on Firefox (Chrome was the non-beta version). I have no reason to believe it won't work on VM stable in FF or VM Beta in Chrome, but I'll double-check before merging.

@kellnerd
Copy link
Collaborator

Well, you already did more tests than I would have done, so I definitely won't complain 😂
I just wanted to make sure that it actually fixes the problem with VM Beta (which I still haven't installed in any browser).

@ROpdebee
Copy link
Owner Author

I really needed to downgrade VM to the stable version anyway to verify whether #465 was because of VM beta. Can confirm this fix works on Chrome/VM beta and FF/VM stable too.

@ROpdebee ROpdebee merged commit 6310f3c into main Jun 12, 2022
@ROpdebee ROpdebee deleted the ecau-new-form-input branch June 12, 2022 10:12
github-actions bot added a commit that referenced this pull request Jun 12, 2022
fix(ecau): drop images without jQuery (#472)
@github-actions
Copy link

🚀 Released 1 new userscript version(s):

  • mb_enhanced_cover_art_uploads 2022.6.12.2 in 7fac2e8

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Violentmonkey Beta fails to find enqueued images
2 participants