Fixes #5359 - Knowledge Base update with big images is not working correctly (image and following content not saved).

Co-authored-by: Dominik Klein <dk@zammad.com>
Co-authored-by: Dusan Vuckovic <dv@zammad.com>

app/assets/javascripts/app/controllers/_ui_element/richtext_additions/_rich_text_tool_popup.coffee

@@ -54,8 +54,8 @@ class App.UiElement.richtext.additions.RichTextToolPopup extends App.ControllerF
     @delegate?.getAjaxAttributes?(field, attributes)
 
   onClear: (e) =>
-    e.preventDefault()
-    e.stopPropagation()
+    e?.preventDefault()
+    e?.stopPropagation()
 
     @clear()
 

app/assets/javascripts/app/controllers/_ui_element/richtext_additions/popup_image.coffee

@@ -3,10 +3,22 @@ class App.UiElement.richtext.additions.RichTextToolPopupImage extends App.UiElem
   labelExisting: 'Replace'
 
   apply: (callback) ->
-    @el.find('btn--create').attr('disabled', true)
+    @el.find('.btn--create').attr('disabled', true)
 
     file = @el.find('input')[0].files[0]
 
+    fileSizeInMb = file.size/1024/1024
+
+    # The browser may fail while reading too large files as data URL.
+    #   Here we introduce a safe limit check in order to prevent silent errors.
+    if fileSizeInMb > 25
+      console.error('App.UiElement.richtext.additions.RichTextToolPopupImage', 'image file size too large', fileSizeInMb, 'in mb')
+      @onClear()
+      new App.ControllerErrorModal(
+        message: __('Image file size is too large, please try inserting a smaller file.')
+      )
+      return
+
     reader = new FileReader()
 
     reader.addEventListener('load', =>
@@ -16,40 +28,48 @@ class App.UiElement.richtext.additions.RichTextToolPopupImage extends App.UiElem
 
     reader.readAsDataURL(file)
 
-  applyOnto: (dom, base64) ->
+  applyOnto: (dom, base64, width) ->
     dom.attr('src', base64)
+    dom.attr('width', width)
 
   insertImage: (base64) ->
     textEditor = $(@event.currentTarget).closest('.richtext.form-control').find('[contenteditable]')
 
-    switch @selection.type
-      when 'existing'
-        @applyOnto(@selection.dom, base64)
-      when 'append'
-        newElem = $('<img>')[0]
-        newElem.src = base64
-        newElem.style = 'width: 1000px; max-width: 100%;'
-        @selection.dom.append(newElem)
-      when 'caret'
-        newElem = $('<img>')
-        newElem.attr('src', base64)
-        newElem.attr('style', 'width: 1000px; max-width: 100%;')
-
-        surroundingDom = @selection.dom[0]
-
-        if surroundingDom instanceof Text
-          @selection.dom[0].splitText(@selection.offset)
-
-        newElem.insertAfter(@selection.dom)
-      when 'range'
-        newElem = $('<img>')
-        newElem.attr('src', base64)
-        newElem.attr('style', 'width: 1000px; max-width: 100%;')
-
-        placeholder = textEditor.find('span.highlight-emulator')
-
-        placeholder.empty()
-        placeholder.append(newElem)
+    insert = (dataUrl, width, height, isResized) =>
+      switch @selection.type
+        when 'existing'
+          @applyOnto(@selection.dom, dataUrl, width)
+        when 'append'
+          newElem = $('<img>')[0]
+          newElem.src = dataUrl
+          newElem.style = 'width: ' + width + 'px; max-width: 100%;'
+          @selection.dom.append(newElem)
+        when 'caret'
+          newElem = $('<img>')
+          newElem.attr('src', dataUrl)
+          newElem.attr('style', 'width: ' + width + 'px; max-width: 100%;')
+
+          surroundingDom = @selection.dom[0]
+
+          if surroundingDom instanceof Text
+            @selection.dom[0].splitText(@selection.offset)
+
+          newElem.insertAfter(@selection.dom)
+        when 'range'
+          newElem = $('<img>')
+          newElem.attr('src', dataUrl)
+          newElem.attr('style', 'width: ' + width + 'px; max-width: 100%;')
+
+          placeholder = textEditor.find('span.highlight-emulator')
+
+          placeholder.empty()
+          placeholder.append(newElem)
+
+    App.ImageService.resize(base64, 1200, 'auto', 2, @getImageType(base64), 'auto', insert)
+
+  getImageType: (base64) ->
+    match = base64.match(/^data:(image\/\w+);base64,/)
+    if match then match[1] else null
 
   clear: ->
     switch @selection.type

app/assets/javascripts/app/controllers/knowledge_base/add_form.coffee

@@ -28,7 +28,7 @@ class App.KnowledgeBaseAddForm extends App.ControllerModal
     params = @formController.paramsForSaving()
     params.translations_attributes[0].content_attributes = { body: '' }
 
-    @parentController.coordinator.saveChanges(@object, params, @)
+    @parentController.coordinator.saveChanges(@object, params, @, e)
 
   showAlert: (text) ->
     @formController?.showAlert(text)

app/assets/javascripts/app/controllers/knowledge_base/content_controller.coffee

@@ -103,7 +103,7 @@ class App.KnowledgeBaseContentController extends App.Controller
     additional_action = $(e.currentTarget).data('id')
 
     if @remoteDidntChangeSinceStart()
-      @parentController.coordinator.saveChanges(@object, paramsForSaving, @, additional_action)
+      @parentController.coordinator.saveChanges(@object, paramsForSaving, @, e, additional_action)
       return
 
     new App.ControllerConfirm(
@@ -111,7 +111,7 @@ class App.KnowledgeBaseContentController extends App.Controller
       message:     __('Your changes may override someone else\'s changes. Do you really want to save?')
       buttonClass: 'btn--danger'
       callback: =>
-        @parentController.coordinator.saveChanges(@object, paramsForSaving, @)
+        @parentController.coordinator.saveChanges(@object, paramsForSaving, @, e)
     )
 
   missingTranslation: ->

app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee

@@ -28,8 +28,20 @@ class App.KnowledgeBaseEditorCoordinator
     else
       object.url()
 
-  saveChanges: (object, data, formController, action) ->
-    App.ControllerForm.disable(formController.form)
+  submitDisable: (e) =>
+    if e
+      App.ControllerForm.disable(e)
+      return
+    App.ControllerForm.disable(@$('.js-submitContainer'), 'button')
+
+  submitEnable: (e) =>
+    if e
+      App.ControllerForm.enable(e)
+      return
+    App.ControllerForm.enable(@$('.js-submitContainer'), 'button')
+
+  saveChanges: (object, data, formController, e, action) ->
+    @submitDisable(e)
 
     url = @urlFor(object) + '?full=true'
 
@@ -40,11 +52,12 @@ class App.KnowledgeBaseEditorCoordinator
       type: object.writeMethod()
       data: JSON.stringify(data)
       url: url
-      success: (data) ->
+      success: (data) =>
         App.Collection.loadAssets(data.assets)
         formController.didSaveCallback(data)
-      error: (xhr) ->
+        @submitEnable(e)
+      error: (xhr) =>
         data = JSON.parse(xhr.responseText)
-        App.ControllerForm.enable(formController.form)
         formController.showAlert(data.error || __('The changes could not be saved.'))
+        @submitEnable(e)
     )

app/assets/javascripts/app/lib/app_post/image_service.coffee

@@ -82,13 +82,26 @@ class App.ImageService
         else
           quality = 0.6
 
+      newType = @validateType(type)
+      newDataUrl = canvas.toDataURL(newType, quality)
+      newSizeInMb = (newDataUrl.length * 0.75)/1024/1024 # approx. file size
+
+      # Nokogiri parser has a single text node limit of 10 million characters, which may be exceeded for certain file
+      #   types very easily (e.g. image/png), even for resized images. Here we check if the new size exceeds this limit
+      #   and prevent execution of the callback (#5359).
+      if newDataUrl.length > 10000000
+        console.error('ImageService', 'image file size too large', newType, newSizeInMb, 'in mb')
+        new App.ControllerErrorModal(
+          message: __('Image file size is too large, please try inserting a smaller file.')
+        )
+        return
+
       # execute callback with resized image
-      newDataUrl = canvas.toDataURL(@validateType(type), quality)
       if resize
-        console.log('ImageService', 'resize', x/sizeFactor, y/sizeFactor, quality, (newDataUrl.length * 0.75)/1024/1024, 'in mb')
+        console.log('ImageService', 'resize', x/sizeFactor, y/sizeFactor, quality, newSizeInMb, 'in mb')
         callback(newDataUrl, x/sizeFactor, y/sizeFactor, true)
         return
-      console.log('ImageService', 'no resize', x, y, quality, (newDataUrl.length * 0.75)/1024/1024, 'in mb')
+      console.log('ImageService', 'no resize', x, y, quality, newSizeInMb, 'in mb')
       callback(newDataUrl, x, y, false)
 
     # load image from data url

app/assets/javascripts/app/lib/base/jquery.contenteditable.js

@@ -350,6 +350,18 @@
         this.log('paste image', clipboardImage)
 
         var imageFile = clipboardImage.getAsFile()
+        var fileSizeInMb = imageFile.size / 1024 / 1024
+
+        // The browser may fail while reading too large files as data URL.
+        //   Here we introduce a safe limit check in order to prevent silent errors.
+        if (fileSizeInMb > 25) {
+          console.error('Image file size too large', fileSizeInMb, 'in mb')
+          new App.ControllerErrorModal({
+            message: __('Image file size is too large, please try inserting a smaller file.'),
+          })
+          return
+        }
+
         var reader = new FileReader()
 
         reader.onload = $.proxy(function (e) {

app/assets/stylesheets/zammad.scss

@@ -1206,6 +1206,10 @@ pre code {
       }
     }
   }
+
+  button[disabled] ~ .dropdown-menu-accessories button {
+    opacity: 0.33;
+  }
 }
 
 .dropdown-menu-right {

i18n/zammad.pot

@@ -7339,6 +7339,11 @@ msgstr ""
 msgid "Image file"
 msgstr ""
 
+#: app/assets/javascripts/app/controllers/_ui_element/richtext_additions/popup_image.coffee:18
+#: app/assets/javascripts/app/lib/app_post/image_service.coffee:95
+msgid "Image file size is too large, please try inserting a smaller file."
+msgstr ""
+
 #: app/frontend/shared/components/CommonFilePreview/CommonFilePreview.vue:107
 msgid "Image of %s"
 msgstr ""
@@ -14069,7 +14074,7 @@ msgstr ""
 #: app/assets/javascripts/app/controllers/_channel/telegram.coffee:211
 #: app/assets/javascripts/app/controllers/_channel/twitter.coffee:300
 #: app/assets/javascripts/app/controllers/knowledge_base/content_can_be_published_dialog.coffee:36
-#: app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee:49
+#: app/assets/javascripts/app/controllers/knowledge_base/editor_coordinator.coffee:61
 #: app/assets/javascripts/app/controllers/knowledge_base/public_menu_form.coffee:68
 #: app/assets/javascripts/app/controllers/knowledge_base/sidebar/attachments.coffee:135
 msgid "The changes could not be saved."

spec/system/knowledge_base/locale/answer/edit_spec.rb

@@ -22,6 +22,36 @@ RSpec.describe 'Knowledge Base Locale Answer Edit', type: :system do
     end
   end
 
+  context 'when image is added via button' do
+    before do
+      image = Rszr::Image.load('spec/fixtures/files/image/large.png')
+      image.resize!(:auto, 30_000)
+      image.save('tmp/really-large.png')
+    end
+
+    def open_editor_and_add_image
+      visit "#knowledge_base/#{knowledge_base.id}/locale/#{primary_locale.system_locale.locale}/answer/#{draft_answer.id}/edit"
+
+      find('a[data-action="insert_image"]').click
+
+      within('.popover-content') do
+        find('input[name="link"]', visible: :all).set(Rails.root.join('tmp/really-large.png'))
+        find('[type=submit]').click
+      end
+    end
+
+    it 'can use big inline image' do
+      open_editor_and_add_image
+
+      click '.js-submit'
+      await_empty_ajax_queue
+
+      click_on 'Edit'
+
+      expect(page).to have_css("img[src='/api/v1/attachments/#{draft_answer.reload.translations.first.content.attachments.first.id}']")
+    end
+  end
+
   context 'add weblink' do
     def open_editor_and_add_link(input)
       visit "#knowledge_base/#{knowledge_base.id}/locale/#{primary_locale.system_locale.locale}/answer/#{draft_answer.id}/edit"