search

froala / meteor-froala

Meteor bindings for the Froala WYSIWYG HTML Editor.
https://froala.com/wysiwyg-editor
MIT License
68 stars 19 forks source link

froala_editor_ie8.min.js is applying to non-IE8 browsers in v1.2.5 #7

Closed rjsmith closed 9 years ago

rjsmith commented 9 years ago

In v1.2.5 of this package, the code in the froala-editor_ie8.min.js is being used and overriding the non-IE8 code in froala-editor.min.js

Steps to reproduce:

  1. Create a Froala Editor instance in a meteor template
  2. Run the Meteor app in a non- IE8 browser (e.g. Chrome v40)
  3. Add a hyperlink in the content (doesn;t matter what it is linked to)
  4. Click on the link in the displayed editor content. Get this error:
Uncaught TypeError: Cannot read property 'type' of undefinedfroala_editor_ie8.min.js:6 $.Editable.saveSelectionfroala_editor.min.js:10 a.Editable.insertLinkfroala_editor.min.js:8 a.Editable.commands.createLink.callbackfroala_editor.min.js:8 a.Editable.execfroala_editor.min.js:10 a.Editable.initLinkEvents.djquery.js:4665 jQuery.event.dispatchjquery.js:4333 jQuery.event.add.elemData.handle

This indicates that code in froala-editor_ie8.min.js is being executed.

Looking at the commit history of froala-editor_ie8.min.js in this repo, it looks like the code to detect IE8 has been removed in v1.2.5, which means the IE8 patches are being applied to all browsers.

stefanneculai commented 9 years ago

The ie8 file should be included only if the browser is lower than IE9 and that should be done in HTML. See https://editor.froala.com/docs, there is a conditional comment for including the IE8 JS file.

rjsmith commented 9 years ago

Hi,

The Meteor package infrastructure does not allow or support polyfills in this way .. all the included js files in a lib folder are automatically added to the generated section of the rendered web page.

I guess that is why the v1.2.4 version of froala-editor_ie8.min.js wrapped the minified code in a function and a browser detection routine.

stefanneculai commented 9 years ago

Indeed, @uzumaxy created the package initially and I was not aware of that. The IE8 conditions from version 1.2.4 JS should be restored.