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

Tinymce update with upstream #7

Closed
wants to merge 3 commits into from
Closed

Conversation

es
Copy link

@es es commented Aug 19, 2014

This PR updates our fork of TinyMCE to the latest upstream master. I made a clean fork of upstream and cherry-picked our commits over. I left out commits that fixed bugs already fixed in upstream. Commits cherry-picked over:

  • Fixes HTML mangling while stripping data-mce helpers (f423a2a)
  • Updated readme (87865ae)
  • Added noparsing plugin (41eff28)
  • Fixed compile error in noparsing plugin (cb4abc5)
  • prevent serialization in setContent (669d841)
  • prevent removal of empty elements in DOM parser (e40de13)
  • Prevent bogus br from being added to empty RTE content (e540911)
  • Set node to current context if undefined (9234e6f)
  • Add optional CSP header to Editor iframe (f51b9c2)

There are some tests that test for things we want/don't want in Admin. I commented them out, but would be happy to just delete them too:

Since this branch rewrites the history of master It'll have to be a force push.

@angelini @nsimmons

@nsimmons
Copy link

Let's merge this once we've tested it out in Shopify and let it run in production for awhile. Just in case this update becomes problematic.

@es
Copy link
Author

es commented Aug 20, 2014

Added CircleCI config so now we're encouraged to modify & track changes with tests.

@es es force-pushed the tinymce-update-with-upstream branch from 80d574c to c72cff4 Compare August 21, 2014 15:16
@EiNSTeiN-
Copy link

I'm a bit late to the party but my commit f51b9c2 was merged upstream already, it didn't need to be cherry-picked, this will cause the 'load' event to be fired twice.

@EiNSTeiN-
Copy link

the double 'load' is because the upstream patch was implemented slightly differently due to upstream code changes.

@es
Copy link
Author

es commented Aug 21, 2014

So can I simply extract your commit or are there changes that need to be done?

@EiNSTeiN-
Copy link

Yes, removing f51b9c2 should be good enough, changes are already in def4d31

@es es force-pushed the tinymce-update-with-upstream branch from c72cff4 to 8935c20 Compare August 25, 2014 15:13
@es
Copy link
Author

es commented Aug 25, 2014

@nsimmons @angelini :shipit:?

@angelini
Copy link

🏇 💨

@nsimmons
Copy link

Everything running pretty smooth so far.

:shipit:

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