From 86d65a571b3abfbb18de8804b9a466d695aa94a2 Mon Sep 17 00:00:00 2001 From: Romit Choudhary Date: Tue, 17 Aug 2021 14:13:41 +0200 Subject: [PATCH] Fixes #3539 - When replying, quote article content only --- .../article_action/email_reply.coffee | 77 +++++++++++++-- .../app/lib/app_post/clipboard.coffee | 27 ++++- .../ticket/update/full_quote_header_spec.rb | 99 ++++++++++++++++++- 3 files changed, 190 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee index 862cb5616..f7bcf3867 100644 --- a/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee +++ b/app/assets/javascripts/app/controllers/ticket_zoom/article_action/email_reply.coffee @@ -127,16 +127,14 @@ class EmailReply extends App.Controller # get current body body = ui.el.closest('.ticketZoom').find('.article-add [data-name="body"]').html() || '' - # check if quote need to be added + # check if quote need to be added via user selection of content signaturePosition = 'bottom' - selected = App.ClipBoard.getSelected('html') - if selected - selected = App.Utils.htmlCleanup(selected).html() - if !selected - selected = App.ClipBoard.getSelected('text') - if selected - selected = App.Utils.textCleanup(selected) - selected = App.Utils.text2html(selected) + + if !@hasUserSelectedContent(ui) + selected = '' + else + selected = @getSelectedContent(ui) + selected = @cleanUpHtmlSelection(selected) # full quote, if needed if !selected && article && App.Config.get('ui_ticket_zoom_article_email_full_quote') @@ -170,6 +168,67 @@ class EmailReply extends App.Controller true + @cleanUpHtmlSelection: (selected) -> + if selected + cleaned_up = App.Utils.htmlCleanup(selected).html() + + return cleaned_up if cleaned_up + + text = App.ClipBoard.getSelected('text') + return App.Utils.text2html(text) if text + + false + + # Fixes Issue #3539 - When replying quote article content only + @getSelectedContent: (ui) -> + range = window.getSelection().getRangeAt(0) + parentSelector = ui.el.closest('.ticket-article-item').attr('id') + + return if !parentSelector + + lastSelElem = $('#' + parentSelector + ' .richtext-content')[0] + + startInsideArticle = @isInsideSelectionBoundary(range.startContainer, parentSelector) + endInsideArticle = @isInsideSelectionBoundary(range.endContainer, parentSelector) + + if !startInsideArticle && endInsideArticle + range.setStart(lastSelElem, 0) + else if startInsideArticle && !endInsideArticle + range.setEnd(lastSelElem, lastSelElem.childNodes.length) + else if @containsNode(lastSelElem) + range.setStart(lastSelElem, 0) + range.setEnd(lastSelElem, lastSelElem.childNodes.length) + + App.ClipBoard.manuallyUpdateSelection() + App.ClipBoard.getSelected('html') + + # checks if user has made any text selection + # checks if that text selection is inside article-content only + @hasUserSelectedContent: (ui) -> + selObject = App.ClipBoard.getSelectedObject() + + if selObject.rangeCount > 0 + # item on which reply is clicked + parentTicketArticleContainer = ui.el.closest('.ticket-article-item') + if parentTicketArticleContainer + parentSelector = parentTicketArticleContainer.attr('id') + range = selObject.getRangeAt(0) + return @isInsideSelectionBoundary(range.startContainer, parentSelector) || @isInsideSelectionBoundary(range.endContainer, parentSelector) || @containsNode($('#' + parentSelector + ' .richtext-content')[0]) + else + return false + + @isInsideSelectionBoundary: (node, parentSelectorId) -> + hasParent = $(node).closest('#' + parentSelectorId + ' .richtext-content') + return hasParent && hasParent.attr('class') is 'richtext-content' + + # Selection.containsNode is not supported in IE, hence check + @containsNode: (node) -> + selected = App.ClipBoard.getSelectedObject() + if typeof selected.containsNode == 'function' + return selected.containsNode(node, false) + else + return false + @date_format: (date_string) -> options = { weekday: 'long' diff --git a/app/assets/javascripts/app/lib/app_post/clipboard.coffee b/app/assets/javascripts/app/lib/app_post/clipboard.coffee index 03dd63b63..004c37c4e 100644 --- a/app/assets/javascripts/app/lib/app_post/clipboard.coffee +++ b/app/assets/javascripts/app/lib/app_post/clipboard.coffee @@ -6,11 +6,21 @@ class App.ClipBoard _instance ?= new _Singleton _instance.bind(el) + @manuallyUpdateSelection: (type) -> + if _instance == undefined + _instance ?= new _Singleton + _instance.manuallyUpdateSelection(type) + @getSelected: (type) -> if _instance == undefined _instance ?= new _Singleton _instance.getSelected(type) + @getSelectedObject: (type) -> + if _instance == undefined + _instance ?= new _Singleton + _instance.getSelectedObject(type) + @getSelectedLast: (type) -> if _instance == undefined _instance ?= new _Singleton @@ -36,9 +46,11 @@ class _Singleton @selection = html: '' text: '' + sel: null @selectionLast = html: '' text: '' + sel: null # bind to fill selected text into bind: (el) -> @@ -59,7 +71,7 @@ class _Singleton ) _updateSelection: => - for key in ['html', 'text'] + for key in ['html', 'text', 'sel'] @selection[key] = @_getSelected(key) if @selection[key] @selectionLast[key] = @selection[key] @@ -86,12 +98,23 @@ class _Singleton for i in [1..sel.rangeCount] container.appendChild(sel.getRangeAt(i-1).cloneContents()) html = container.innerHTML - html + + if type != 'sel' + html + else + sel + + manuallyUpdateSelection: -> + @_updateSelection() # get current selection getSelected: (type) -> @selection[type] + # get current selection original object + getSelectedObject: -> + @selection['sel'] + # get latest selection getSelectedLast: (type) -> @selectionLast[type] diff --git a/spec/system/ticket/update/full_quote_header_spec.rb b/spec/system/ticket/update/full_quote_header_spec.rb index aaef3e437..b3fd84b6d 100644 --- a/spec/system/ticket/update/full_quote_header_spec.rb +++ b/spec/system/ticket/update/full_quote_header_spec.rb @@ -8,6 +8,7 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr let(:ticket_article) { create(:ticket_article, ticket: ticket, from: 'Example Name ') } let(:customer) { create(:customer) } let(:current_user) { customer } + let(:selection) { '' } prepend_before do Setting.set 'ui_ticket_zoom_article_email_full_quote_header', full_quote_header_setting @@ -107,6 +108,78 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr end end + context 'when text is selected on page while replying' do + let(:full_quote_header_setting) { false } + let(:before_article_content_selector) { '.ticketZoom-header' } + let(:after_article_content_selector) { '.ticket-article-item .humanTimeFromNow' } + let(:article_content_selector) { '.ticket-article-item .richtext-content' } + + it 'does not quote article when bits other than the article are selected' do + within(:active_content) do + selection = highlight_and_get_selection(before_article_content_selector, '') + click_reply + + within(:richtext) do + expect(page).to have_no_text(selection) + end + end + end + + it 'quotes article when bits inside the article are selected' do + within(:active_content) do + selection = highlight_and_get_selection(article_content_selector, '') + click_reply + + within(:richtext) do + expect(page).to have_text(selection) + end + end + end + + it 'quotes only article when bits before the article are selected as well' do + within(:active_content) do + selection = highlight_and_get_selection(before_article_content_selector, article_content_selector) + expected_text = find(article_content_selector).text + + click_reply + + within(:richtext) do + expect(page).to have_no_text(selection) + expect(page).to have_text(expected_text) + end + end + end + + it 'quotes only article when bits after the article are selected as well' do + within(:active_content) do + selection = highlight_and_get_selection(article_content_selector, after_article_content_selector) + expected_text = find(article_content_selector).text + + click_reply + + within(:richtext) do + expect(page).to have_no_text(selection) + expect(page).to have_text(expected_text) + end + end + end + + it 'quotes only article when bits both before and after the article are selected as well' do + within(:active_content) do + selection = highlight_and_get_selection(before_article_content_selector, after_article_content_selector) + expected_text = find(article_content_selector).text + + click_reply + + within(:richtext) do + expect(page).to have_no_text(selection) + expect(page).to have_text(expected_text) + end + end + end + + end + def click_forward click '.js-ArticleAction[data-type=emailForward]' end @@ -115,8 +188,30 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr click '.js-ArticleAction[data-type=internal]' end + def click_reply + click '.js-ArticleAction[data-type=emailReply]' + end + + def highlight_and_get_selection(start_selector, end_selector) + find(start_selector) + .execute_script(<<~JAVASCRIPT, end_selector) + let [ end_selector ] = arguments + let end_node = $(end_selector)[0] + if(!end_node) { + end_node = this.nextSibling + } + window.getSelection().removeAllRanges() + var range = window.document.createRange() + range.setStart(this, 0) + range.setEnd(end_node, end_node.childNodes.length) + window.getSelection().addRange(range) + JAVASCRIPT + + find(start_selector).evaluate_script 'window.getSelection().toString().trim()' + end + def highlight_and_click_reply - find('.ticket-article-item .textBubble') + find('.ticket-article-item .richtext-content') .execute_script <<~JAVASCRIPT window.getSelection().removeAllRanges() var range = window.document.createRange() @@ -125,7 +220,7 @@ RSpec.describe 'Ticket > Update > Full Quote Header', current_user_id: -> { curr window.getSelection().addRange(range) JAVASCRIPT - click '.js-ArticleAction[data-type=emailReply]' + click_reply end define :contain_full_quote do