Closed bregenspan closed 9 years ago
Brilliant! It's been in the back of my mind for a while but didn't really have the use case. Let's try and get the test harness released first and then we'll look at the test suite.
Looks great! :+1:
@bregenspan We tried merging but couldn't get the test suite to work on Chrome. No content seems to get pasted to Scribe. We weren't sure how to progress, do you have an idea of what might be wrong?
@rrees argh, sorry about that! Worked for me locally in Chrome Version 41.0.2272.3 dev on OSX but because this involves focus/selections and there are so many weird Webdriver focus-related issues, I should've checked against Travis first. I might not have time to fix up today but will do ASAP, in meantime seeing what Travis says here https://travis-ci.org/gawkermedia/scribe-1/jobs/47131815
So this was completely the fault of me relying on undocumented behavior that is apparently only present in the latest versions of Chrome (ability to set custom properties to an event), and not something exotic like Webdriver focus issues. I can get things working by adding one solution that works for Chrome, one for FF, but have pretty mixed feelings about adding something that hacky.
An alternate idea is to introduce a little bit of ugliness in the paste handling by having it look for event.clipboardData || event.detail.clipboardData
(the latter can safely be set in a custom event), but it's definitely a choice between two evils here, I'll keep thinking about it.
Thanks for adding some tests to this, @bregenspan! Yeah it looks like this is the root of the chrome (and probably safari) problems: https://github.com/WebKit/webkit/blob/master/Source/WebCore/dom/Event.idl#L77-L79
@awentzonline looking at the source is cheating!
So IMO the correct, least-hacky way of handling this is to go back to keyboard events like: https://github.com/gawkermedia/scribe-test-harness/compare/add-paste-helper-keyboard-version-3?expand=1 and just conditionally not execute paste-related tests on the OSX * Chrome combination until the Chromedriver bug with command keys is fixed. That way there's still full coverage in Travis and local test-running on OSX still behaves at least in FF.
If that sounds good to everyone I'll try to fix the remaining glitches in that approach (e.g. https://travis-ci.org/gawkermedia/scribe-1/jobs/47267311) as soon as I can (but anyone should feel free to jump in if you have time and want to before then).
Sorry again for the fail to test outside of dev version of Chrome!
@awentzonline @bregenspan @hmgibson23 and I we're discussing this the other day and the way we think is easiest to go forward is that we should bind the native cut and paste events to scribe internal events and have the paste functionality tests be based on the internal event.
We'll lack some coverage in the native event binding to the internal events but everything else would be reliably testable.
Let us know if you think this will work.
@rrees I like that, feels like a clear way! Means some paste event object might have to be mocked or there will need to be separate events for HTML vs plaintext but avoids this whole Selenium nightmare.
This is all @awentzonline's work (thank you!!), just with the addition of a test. This should resolve #241 if all looks good. It depends on https://github.com/guardian/scribe-test-harness/pull/10 being merged and released as it needs a new helper method for inserting via paste (if it's simpler, I could instead inline that for now).
Because the example paste formatter used for the test is kind of a one-off, I wasn't totally sure where to put it -- it lives in "examples" for now.