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

New empty lines appearing unexpectedly in long text contents #446

Closed
bago opened this issue May 4, 2018 · 4 comments · Fixed by #644
Closed

New empty lines appearing unexpectedly in long text contents #446

bago opened this issue May 4, 2018 · 4 comments · Fixed by #644
Assignees

Comments

@bago
Copy link
Member

bago commented May 4, 2018

Since 0.17 we have reports of people finding empty newlines in unexpected places.

From investigation it seems related to a bug with Tinymce < 4.7 since we switched to using "getContent" instead of "getContent({format: raw})".

Here is a WiP fiddle
http://fiddle.tinymce.com/4rgaab

If we remove our "BeforeSetContent" everything works again, but the "trim issue" will come back, then.

@bago bago added this to the 0.18 milestone May 4, 2018
@bago bago self-assigned this May 4, 2018
@bago
Copy link
Member Author

bago commented May 4, 2018

So, tinymce 4.7+ would fix this, but we can't upgrade to 4.7+ because of #429 that is introduced by 4.7+

@bago
Copy link
Member Author

bago commented May 4, 2018

Submitted the issue to tinymce:
tinymce/tinymce#4382

@bago
Copy link
Member Author

bago commented May 4, 2018

Sounds like this is "minor" in the official release because the bug is almost "invisible" if you use the default options.

On the otherside, if you use the experimental feature "ko.bindingHandlers.wysiwyg.getContentOptions = {}", then the buggy elements become visible and you see the unexpected lines. The unexpected lines are still there without this option, but are 0px height and you don't see them.

bago added a commit that referenced this issue Jul 25, 2018
Hiswe added a commit to abernier/mosaico-backend that referenced this issue Sep 1, 2018
* More conditions for scrollfix.js

Fixes voidlabs#374 . Should work with any 4+ version of tinymce.

* Dependency updates + ko 3.4.2 compatibility

Updated build/dev (npm/bower) dependencies + add ko 3.4.2 support for
subscriptions tracking.

* Modified README with link to ASP.NET Core backend

* Modified README with link to ASP.NET MVC 5 backend

* Revert ko-reactor to 1.3.9 + Embed ko-undomanager and ko-wrap

Fixes voidlabs#407 and voidlabs#408

* Move away from bower. Switch to plain npm

Fix voidlabs#409

* Move away from bower. Switch to plain npm

Fix voidlabs#409

* Added reference to Rails engine

* Force jquery-ui 1.11 because of jquery-ui 1.12 not being compatible

There are quirks with jquery ui 1.12 that introduced incompatibilities and knockout-jqueryui doesn't deal with them.

* Fix specs according to bower removal refactoring

* Fixes for bower removal

* Minor change trying to debug a weird issue with npm on CI tools

* Update dev dependencies

* Fix travis declaration (we don't support node 5, but we support node 6, 7 and 8)

* Use jquery3+jquery migrate instead of jquery 1.12 (security issues)

* Forgot a change in the jquery 3 upgrade

* Depdendencies update (ko 3.4.2, reactor 1.4.0, ko-sortable 1.0.0)

Support for ko 3.4.2 and ko 3.5.0-beta (both in our code and by upgrading ko-reactor to latest 1.4.0 release).

Update to jquery ui 1.12 includes even if we stick to jquery ui 1.11 for compatibility reasons so we can easily bundle the latest ko-sortable version (that moved to the newest release)

Another attempt to fix voidlabs#379

* Updated jasmine to 3.0.0 + added grunt clean task

* Basic wrapper/undomanager spec

Include commented code for "known failing tests"

* Replace knockout-wrap with a simple adhoc function

Fix voidlabs#408

* grunt-contrib-clean is a dev dependency, not a runtime dependency

* build fixes + package-lock update

* Update versafix to 1.1.16 release from the versafix-template project

Versafix now have its own repository:
https://github.com/voidlabs/versafix-template

* Move to official evol-colorpicker npm (new releases merged our PR, so we don't need a fork anymore)

+ also fixed a comment in the colorpicker integration code.
Complete the fix for #3

* Added community links to README

* Reduce logging + Prepare 0.17 release

Translated a missed italian log, moved to console.debug for some very verbose logging.

* Logging improvements

* Switch back from console.debug to console.log

Console.debug is not available in node < 9 nor in some browsers.

* Add license banner + Grunt compress task to make the binary release asset

Also simplify the browserify build to avoid double builds.

* Show debug where console.debug is available, otherwise mute it.

* Automatically inject package version number in the code during the build

* Added grunt release task

* Testing grunt release plugin

* release 0.17.0

* Explicit bin naming for the release assets

Skip npm publishing during release.

* New license_check grunt goal + new dist goal.

* Fixes with test popup prompt

1) Edge Metro doesn't have a prompt method
2) When closing the window without email addresses this produced an error and stuck the button.

* Minor fixes for package.json

* Minor fixes to grunt release and lib minification size

* Merge external dependencies to a single js+css file

* Make "dist" folder more comprehensive

* Moved index/editor.html to src folder

* use passive events by default + sourcemaps for dependencies bundle

* wysiwyg/tinymce: add debug option + default to "br" newlines on short texts.

* Rename droppable binding to avoid name clash

knockout-sortable 1.1.0 introduced its own droppable binding. It is not compatible with our needs, so we prefer to rename our binding, instead of overwriting the original binding with our code.

* Fast fail on unsupported jqueryui version

jquery ui 12 introduced backward incompatible changes. Our dependency (knockout-jqueryui) only supports 1.08 to 1.11, so we stick to 1.11

* we now supprot latest ko-sortable (bump devdependencies)

* Attempt to catch some tinymce exceptions

* DOWNGRADE tinymce to 4.5.8 + Fix for ko-sortable 1.1.0 support

We experienced major random issues with TinyMCE 4.6.0+ (including latest 4.7.9), so we downgrade to the latest "stable" release.

* Update README.md

* Fix order for deps being bundled

Fix voidlabs#421
Also removed explicit dependency on load-image/canvas-to-blob as we get them via file-upload.

* Support quoted strings in template language declarations

Fix voidlabs#391

* revert test changes

* deal with both JSON and JS for the data parameter during startup

* avoid use deprecated properties to detect webkit

* use debug for expected exceptions

* trim options key/labels for select widget

* Missing localization string for gallery paginator

* placeholder for pt_BR lang

* Update mosaico-nl.json (POEditor.com)

* Update mosaico-en.json (POEditor.com)

* Update mosaico-fr.json (POEditor.com)

* Update mosaico-de.json (POEditor.com)

* Update mosaico-it.json (POEditor.com)

* Update mosaico-ru.json (POEditor.com)

* Update mosaico-sr_RS.json (POEditor.com)

* Update mosaico-es.json (POEditor.com)

* Update mosaico-sv.json (POEditor.com)

* Update mosaico-pt_BR.json (POEditor.com)

* Some 3rd party CI conf (appveyor + circleci)

More CI, more environments.

* Trim option keys while dealing with variants

* Cleanup

* Update versafix-1 parsing test + basic variant test

* Coverage test with travisCI

* Testing with travis and codecov

* Docs refactoring

* "dist" is better than "bin"

* release 0.17.1

* Revert build optimization to keep npm3 compatibility

Fix voidlabs#427

* Minor style fixes (sync upstream)

* tinymce binding improvements

You can now configure:
"ko.bindingHandlers.wysiwyg.getContentOptions: {};" in order to use standard getContent instead of "raw" getContent (it could work better) with regard to empty contents (see voidlabs#428 and voidlabs#426).
ko.bindingHandlers.wysiwyg.useTarget to skip creating id attributes for every editable element and run tinymce against the dom element instead of the selector.
Added ko.bindingHandlers.wysiwyg.initializingClass defaulting to "wysiwyg-loading" that will add the class to the element during the tinymce initialization. Also added some CSS to prevent the editable to be clickable during initialization and show a fade animation in that timeframe.

* wywisyg: more options

- new ko.bindingHandlers.wysiwyg.removeSelectionOnBlur option defaulting to "true" that will force browser selection to be emptied when you unfocus an editor (set this to false to revert to the old behaviour).
- new ko.bindingHandlers.wysiwyg.emptyClass option that will add an "wysiwyg-empty" class on contents with no "text" (remove html tags and trims to check this), so you can add CSS to add behaviour. By default no styles are added to this.
- new debug console error when wysiwyg is started on inline elements (inline-block or block support is better)

* Default to inline-block for editable contents and outline them on hover

- Also changed the spacing for mid-tools (editing icons for images).
- Force cursor icon for empty editables.

* Added image tool to browser the gallery

Set viewModel.selectImage to undefined if you want to remove the new icon.
Override viewModel.selectImage to your own function if you want to change the behaviour of this icon.

* Update mosaico-nl.json (POEditor.com)

* Update mosaico-en.json (POEditor.com)

* Update mosaico-fr.json (POEditor.com)

* Update mosaico-de.json (POEditor.com)

* Update mosaico-it.json (POEditor.com)

* Update mosaico-pt_BR.json (POEditor.com)

* Update mosaico-ru.json (POEditor.com)

* Update mosaico-sr_RS.json (POEditor.com)

* Update mosaico-es.json (POEditor.com)

* Update mosaico-sv.json (POEditor.com)

* trivial: package.lock was not updated according to npm3 compatibility refactoring

* Trying to prevent inline-editable to collapse when empty

We use \U2063 that is the unicode for the "invisible separator"/"invisible comma".

* Move debug about inline editable after tinymce have initialized

It raised false positives when ran before.

* extract unused bindings to their own file + extracted preloader to its own file, so to shring fileupload bindings.

* release 0.17.2

* Temporary workaround for voidlabs#430

Simply revert latest CSS changes for webkit browsers (Safari/Midori..).

Those browsers won't show the outline on editable contents while hovering the blocks.

* Avoid opacity "on hover" in Safari/Midori

Attempt to fix voidlabs#430

* Refactor hover/selection effects to deal with webkit (safari) issues

Trying to workaround voidlabs#430

* minor visual tweak for selections

* prevent double tooltips on image upload button (safari/edge)

Hopefully fix voidlabs#432

* Prevent downloading resources while parsing the master template

Attempt to fix voidlabs#433 by using jQuery(jQuery.parseHTML()) instead of jQuery().
This also require jQuery 3+ (on older jquery it will work, but it will temporarily try to download remote resources from invalid urls)

* Move makeEditableHover to style_mosaico.less near the makeSelectable

* release 0.17.3

* Rename dependencies bundle to fix a tinymce issue in IE11

Tinymce tries to find the path for skin resources on its own. On IE11 the "correct" method fails and it tries to detect the folder by searching the tinymce script urls. But we bundled it and it cannot find itself.. so if we name it so it ends with tinymce.min.js this will "fool" it and fix it. Fix voidlabs#459

* Update versafix to 1.1.17

Fix for voidlabs#467

* Add a comment about a broken option

See voidlabs#446

* Update/improve dependencies

Declare node/npm dependencies
Add NOTES files about dependencies not updated by purpose (and why)
Update other dependencies
Hopefully fixes voidlabs#454

* Prepare for 0.17.4 release / downgrade csswring to 6.0.3 to keep node 6/7 compat

csswring 7.0.0 broke the build under older node versions.

* Fix lock file for downgraded csswring

* NODE – update version

* DEPENDENCIES – update patch/minor

* PRETTIER – add

* DEPENDENCIES – update and fix code

* DEPENDENCIES – add back mosaico's editor's one

* DEPENDENCIES – remove bower

* BUILD – can do again \O/

* DEPENDENCIES – update

* I18N – fix some

* DOWNLOAD – first step of making a download without the images

* ICONS – fix some

* DOWNLOAD – implement CDN images

* ICONS – fix more

* ICONS – fix some more

* DEPENDENCIES – update

* DEPENDENCIES – update font-awesome

* PREVIEW – begin to add on smaller devices

* PREVIEW – change icon when active

* BUILD – fix tinymce plugins

* PREVIEW – fix edit zone resizing on small device

* PREVIEW – highlight current item selected

* BUILD – forcing “npm install” should be obsolete: no more use of bower

* README – update

* RELEASE – keep bin folder

* HEROKU – revert back dependencies install for installing in heroku

* HEROKU – add back gulpfile for building

* HEROKU – add back src folder
@bago
Copy link
Member Author

bago commented Jun 16, 2020

Further tinymce note: since 5.0 tinymce is no more able to make a "td" editable: we already have custom code in the parser to make sure when a td is editable we create a nested div to make it editable, but it is important to note that since tinymce 5.0 that wrapping will be mandatory.

@bago bago removed this from the 0.18 milestone Apr 27, 2022
bago added a commit that referenced this issue May 10, 2022
We found that most browser/tinymce bugs related to contenteditable are about editing non-DIV elements, so we try to change the template converter to always create an utility DIV and make it editable (Fix #429 and #362).

This should still work with 4.5.x (currently the last version supported by mosaico) and by updated releases.

Mosaico now supports multiple editing styles: the legacy styles are named "singleline" and "multiline" and they are automatically applied depending on the data-ko-editable element (td & div use "multiline", the other ones "singleline") but a data-ko-editor-style="newstyle" can be used to force the use of a different tinymce options set. This set will inherit ko.bindingHandlers.wysiwyg.standardOptions, ko.bindingHandlers.wysiwyg.fullOptions and then ko.bindingHandlers.wysiwyg.extendedOptions. (Fix #244)

This changeset also remove the support for ko.bindingHandlers.wysiwyg.getContentOptions and replaces it with a new option "_use_raw_format" that defines wether to use "raw" format when getting/setting the content from tinymce. The new default is to use "raw" format for the singleline style and non raw format for everything else (the raw in singleline is required in order to prevent tinymce from trimming contents). Note that previously mosaico used raw format for every style, so if you want to preserve the previous behaviour you need to set ko.bindingHandlers.wysiwyg.fullOptions._use_raw_format to true. (should fix #446)

Another breaking change is the default  "extended_valid_elements" option that changed from "strong/b,em/i,*[*]" to "strong/b,em/i" so to enable content filtering and strip unwanted tags (like scripts). You can revert it by setting that option to the old value but beware XSS vulnerabilities.

This changeset enable to upgrade of tinymce to newer 4.x (4.7.x, 4.9.x) and latest 5.x (Fix #593). For 5.x and 6.x we also had to update the scrollfix binding so that the toolbar is correclty moved when the edit area is scrolled.

TinyMCE 6.x compatibility requires more changes because they removed support for "forced_root_block = false" or "forced_root_block = ''" that is needed for the singleline editing. You can trick tinymce by usign "forced_root_block = 'unknownelementname'" but it will prevent use of "ENTER" to insert <br/>.

Issues #596 and #443 may be fixed by this changeset.
@bago bago closed this as completed in #644 May 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant