PerfectlyNormal / tinymce-rails-imageupload

Image upload plugin for TinyMCE and Rails with the asset pipeline
MIT License
149 stars 153 forks source link

Bug on Win7 IE9 #29

Closed pomartel closed 11 years ago

pomartel commented 11 years ago

Was it tested on IE9? When I try the plugin, it throws an exception when parsing the JSON in the handleResponse function. It's as if the document contains the html code of the main iframe instead of the hidden_upload iframe. Has anyone run into that issue?

PerfectlyNormal commented 11 years ago

I know it has been tested in IE9, but not sure if the most recent changes have been tested too much.

I'll look into it tomorrow if I get time.

Sent from my iPhone

On 16 apr 2013, at 10:05 em, Pierre Olivier Martel notifications@github.com wrote:

Was it tested on IE9? When I try the plugin, it throws an exception when parsing the JSON in the handleResponse function. It's as if the document contains the html code of the main iframe instead of the hidden_upload iframe. Has anyone run into that issue?

\ Reply to this email directly or view it on GitHub.

pomartel commented 11 years ago

Were you able to take a look at it? I would like to submit a patch request but I'm a bit mystified about how this all works with the ajax request being made in an iframe.

PerfectlyNormal commented 11 years ago

Ahh, sorry, didn't get time wednesday, and then I totally forgot about it! I'll have a look now.

In the meantime, this looks like an OK explanation for what's going on with the iframe: http://www.alfajango.com/blog/ajax-file-uploads-with-the-iframe-method/

pomartel commented 11 years ago

Good to know! I don't know if you have seen in the comments where Steve answers a question about a problem with IE. Maybe that's a starting point? By the way, I also had a problem with Firefox triggering an error when the plugin loads (before submitting a file). I will see if I can still reproduce it now.

PerfectlyNormal commented 11 years ago

I made the branch browser-compatibility with some fixes. Looks fine in IE10 and latest Firefox for me. Looks like IE9 still breaks…

pomartel commented 11 years ago

It is still broken for me in IE8 and IE9, even after clearing the cache. I'm testing with Win XP and Win 7 on Parallels Desktop. I can't test IE10 unfortunately.

PerfectlyNormal commented 11 years ago

Yeah, the cache bit only helped for the initial "Invalid data from the server" error. Pushed another commit that seems to fix IE9 for me. Still working on IE8

PerfectlyNormal commented 11 years ago

Now IE8 worked as well, without further changes. But, dinner time. Will look more in a while (or maybe tomorrow)

pomartel commented 11 years ago

Thanks for the awesome support! I know how supporting IE, especially older versions, can be a pain in the ass... I will test it out now, still far from dinner time in Montreal!

I'm also preparing pull requests for spanish and german translations for the plugin.

PerfectlyNormal commented 11 years ago

Unfortunately, we use the plugin for work, and a few clients demand support for IE8, so I should really have taken better care of this anyhow. And yes, supporting IE is one of my least favorite activities. Looking forward to the day we can drop it, or at least require IE9+.

pomartel commented 11 years ago

I hate to say this but it's still not working for me. Are you sure you pushed your last commit on the browser-compatibility branch?

PerfectlyNormal commented 11 years ago

Latest commit was https://github.com/PerfectlyNormal/tinymce-rails-imageupload/commit/269606a59295b1740541a2015eaaaadc32cfa50e and that was pushed a while ago, unfortunately. Now I got an error when retrying, so looks like it wasn't really fixed after all.

I'll look some more tomorrow (10pm here now).

pomartel commented 11 years ago

Ok, no worries. I don't have much on my plate today so I'll get my hands dirty. With a little luck, you will have the fix in a pull request tomorrow!

PerfectlyNormal commented 11 years ago

That would certainly be appreciated. I got some CSRF-related errors as well while testing (#33). Might be it doesn't like reusing the token between attempts at attaching an image. Haven't looked too much at it yet, just put it there as a note to myself

PerfectlyNormal commented 11 years ago

Released v3.5.8.4 with these fixes, and hope everything works in all the main browsers again. Thanks for the help!