/otwarchive

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

AO3-5033 Typo in minified TinyMCE file prevented it from loading #2987

Merged
merged 1 commit into from Aug 7, 2017

Conversation

Reviewers

@houndci-bot houndci-bot

Assignees
No one assigned
Labels
Reviewed: Ready to Merge
Projects
None yet
Milestone
No milestone
3 participants
@sarken @elzj @houndci-bot
@sarken
Owner

sarken commented Aug 7, 2017

Issue

https://otwarchive.atlassian.net/browse/AO3-5033

Purpose

In the Rails 5.0 pull request, the minified TinyMCE file was modified to add a newline at the end of the file. However, it also introduced a typo into the file which, which prevented TinyMCE from loading. Codeship's poltergeist settings caught this error, causing our tests to fail. This pull request restores the old, typo-free version of the JS file, meaning TinyMCE will work again and our tests will pass, so we can deploy Rails 5.0 to staging.
@sarken
AO3-5033 Typo in minified TinyMCE file prevented it from loading, cau… 
…sing a failure on Codeship and preventing Rails 5.0 from being deployed to test

All checks have passed

3 successful checks
Scrutinizer — 2 new issues
Details
continuous-integration/travis-ci/pr — The Travis CI build passed
Details
hound — 14 violations found.
c1b6edb

@sarken sarken added the Awaiting review label Aug 7, 2017

public/javascripts/mce_editor.min.js
@@ -1 +1 @@
-ounction addEditor(e){tinyMCE.execCommand("mceAddEditor",false,e)}function removeEditor(e){tinyMCE.execCommand("mceRemoveEditor",false,e)}tinyMCE.init({plugins:"directionality hr image link paste tabfocus",menubar:false,toolbar:"bold italic underline strikethrough | link unlink image | blockquote | hr | bullist numlist | alignleft aligncenter alignright alignjustify | undo redo | ltr rtl",force_p_newlines:true;debugger;browser_spellcheck:true,inline_styles:false,convert_urls:false,content_css:"/stylesheets/tiny_mce_custom.css?"+(new Date).getTime(),tabfocus_elements:"tinymce",extended_valid_elements:"b, i, span[!class|!dir], strike, u",invalid_elements:"font",formats:{alignleft:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"left"}},aligncenter:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"center"}},alignright:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"right"}},alignjustify:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"justify"}},underline:{inline:"u",exact:true},strikethrough:[{inline:"strike",exact:true},{inline:"s",remove:"all"},{inline:"del",remove:"all"}]},paste_word_valid_elements:"@[align],-strong/b,-em/i,-u,-span,-p,-ol,-ul,-li,-h1,-h2,-h3,-h4,-h5,-h6,-table,-tr,-td[colspan|rowspan],-th,-thead,-tfoot,-tbody,-a[href|name],sub,sup,strike,br",target_list:[{title:"None",value:""}]});$j(document).ready(function(){$j(".rtf-html-switch").removeClass("hidden");$j(".html-link").addClass("current");$j(".rtf-link").click(function(e){addEditor("content");$j(this).addClass("current");$j(".rtf-notes").removeClass("hidden");$j(".html-link").removeClass("current");$j(".html-notes").addClass("hidden");e.preventDefault()});$j(".html-link").click(function(e){removeEditor("content");$j(this).addClass("current");$j(".html-notes").removeClass("hidden");$j(".rtf-link").removeClass("current");$j(".rtf-notes").addClass("hidden");e.preventDefault()})})
+function addEditor(e){tinyMCE.execCommand("mceAddEditor",false,e)}function removeEditor(e){tinyMCE.execCommand("mceRemoveEditor",false,e)}tinyMCE.init({plugins:"directionality hr image link paste tabfocus",menubar:false,toolbar:"bold italic underline strikethrough | link unlink image | blockquote | hr | bullist numlist | alignleft aligncenter alignright alignjustify | undo redo | ltr rtl",browser_spellcheck:true,inline_styles:false,convert_urls:false,content_css:"/stylesheets/tiny_mce_custom.css?"+(new Date).getTime(),tabfocus_elements:"tinymce",extended_valid_elements:"b, i, span[!class|!dir], strike, u",invalid_elements:"font",formats:{alignleft:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"left"}},aligncenter:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"center"}},alignright:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"right"}},alignjustify:{selector:"div, h1, h2, h3, h4, h5, h6, img, p, table, td, th, ul, ol, li",attributes:{align:"justify"}},underline:{inline:"u",exact:true},strikethrough:[{inline:"strike",exact:true},{inline:"s",remove:"all"},{inline:"del",remove:"all"}]},paste_word_valid_elements:"@[align],-strong/b,-em/i,-u,-span,-p,-ol,-ul,-li,-h1,-h2,-h3,-h4,-h5,-h6,-table,-tr,-td[colspan|rowspan],-th,-thead,-tfoot,-tbody,-a[href|name],sub,sup,strike,br",target_list:[{title:"None",value:""}]});$j(document).ready(function(){$j(".rtf-html-switch").removeClass("hidden");$j(".html-link").addClass("current");$j(".rtf-link").click(function(e){addEditor("content");$j(this).addClass("current");$j(".rtf-notes").removeClass("hidden");$j(".html-link").removeClass("current");$j(".html-notes").addClass("hidden");e.preventDefault()});$j(".html-link").click(function(e){removeEditor("content");$j(this).addClass("current");$j(".html-notes").removeClass("hidden");$j(".rtf-link").removeClass("current");$j(".rtf-notes").addClass("hidden");e.preventDefault()})})
@houndci-bot

houndci-bot Aug 7, 2017

Line is too long.
Missing semicolon.
Identifier 'browser_spellcheck' is not in camel case.
Identifier 'inline_styles' is not in camel case.
Identifier 'convert_urls' is not in camel case.
Identifier 'content_css' is not in camel case.
Missing '()' invoking a constructor.
Identifier 'tabfocus_elements' is not in camel case.
Identifier 'extended_valid_elements' is not in camel case.
Identifier 'invalid_elements' is not in camel case.
Identifier 'paste_word_valid_elements' is not in camel case.
Identifier 'target_list' is not in camel case.
'tinyMCE' is not defined.
'$j' is not defined.

@sarken
Owner

sarken commented Aug 7, 2017

Hold that thought. This might be the wrong version of the file...
@sarken
Owner

sarken commented Aug 7, 2017

Nope, it's fine!

Here's what hung me up: the version of this file from the Rails 5.0 PR contained the following:

rtl",force_p_newlines:true;debugger;browser_spellcheck:true,

But our previous master contained:

rtl",browser_spellcheck:true,

The version that was on master (and is in this pull request) is correct.
@elzj
Owner

elzj commented Aug 7, 2017

So we want to remove the line rather than fix the typo? Would 'force_p_newlines' add anything useful?
@sarken
Owner

sarken commented Aug 7, 2017

Oh, sorry! The typo I'm fixing is replacing ounction with function.

The force_p_newlines bit was weird -- I didn't notice it until I had submitted this pull request and was looking at the diff, hence my earlier comments. There's no value to adding it, as far as I could tell. The official documentation (http://archive.tinymce.com/wiki.php/Configuration:force_p_newlines) says it relates to paragraph creation in Firefox, so I tested this branch against what's currently on production, and there's no change in behavior.

On both this branch and production, pressing Return/Enter produces this:

<p>hello</p>
<p>it's me</p>
<p>your friendly paragraph</p>

And pressing Shift+Return/Enter produces this:

<p>a friendly paragraph<br />test</p>
@elzj
Owner

elzj commented Aug 7, 2017

Okay, then, it looks good to me!

@elzj elzj added Reviewed: Ready to Merge and removed Awaiting review labels Aug 7, 2017

@elzj elzj merged commit 5ec9713 into otwcode:master Aug 7, 2017
3 checks passed

3 checks passed

Scrutinizer 2 new issues
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
hound 14 violations found.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment