From 8f384bb4e87503b1e1f8f2d6a4d2d7bef8d55712 Mon Sep 17 00:00:00 2001 From: Evan Morikawa Date: Thu, 28 Jan 2016 17:24:29 -0800 Subject: [PATCH] fix(quoted): fix quoted text issue with single blockquotes Fixes #1084 Also fixes broken tokenizing text field specs Also protects `MessageBodyExtension`s from errors --- .../tokenizing-text-field-spec.cjsx | 13 +- spec/fixtures/emails/email_17.html | 140 ++++++++++++++++++ spec/fixtures/emails/email_17_stripped.html | 94 ++++++++++++ spec/quoted-html-transformer-spec.coffee | 33 ++++- src/flux/stores/message-body-processor.coffee | 13 +- src/services/quoted-html-transformer.coffee | 7 +- 6 files changed, 285 insertions(+), 15 deletions(-) create mode 100644 spec/fixtures/emails/email_17.html create mode 100644 spec/fixtures/emails/email_17_stripped.html diff --git a/spec/components/tokenizing-text-field-spec.cjsx b/spec/components/tokenizing-text-field-spec.cjsx index a130e589a..7143d0622 100644 --- a/spec/components/tokenizing-text-field-spec.cjsx +++ b/spec/components/tokenizing-text-field-spec.cjsx @@ -295,22 +295,29 @@ describe 'TokenizingTextField', -> expect(@tabDownEvent.stopPropagation).toHaveBeenCalled() describe "when blurred", -> + it 'should do nothing if the relatedTarget is null meaning the app has been blurged', -> + ReactTestUtils.Simulate.focus(@renderedInput) + ReactTestUtils.Simulate.change(@renderedInput, {target: {value: 'text'}}) + ReactTestUtils.Simulate.blur(@renderedInput, {relatedTarget: null}) + expect(@propAdd).not.toHaveBeenCalled() + expect(ReactTestUtils.scryRenderedDOMComponentsWithClass(@renderedField, 'focused').length).toBe(1) + it 'should call add, allowing the parent component to (optionally) turn the entered text into a token', -> ReactTestUtils.Simulate.focus(@renderedInput) ReactTestUtils.Simulate.change(@renderedInput, {target: {value: 'text'}}) - ReactTestUtils.Simulate.blur(@renderedInput) + ReactTestUtils.Simulate.blur(@renderedInput, {relatedTarget: document.body}) expect(@propAdd).toHaveBeenCalledWith('text', {}) it 'should clear the entered text', -> ReactTestUtils.Simulate.focus(@renderedInput) ReactTestUtils.Simulate.change(@renderedInput, {target: {value: 'text'}}) - ReactTestUtils.Simulate.blur(@renderedInput) + ReactTestUtils.Simulate.blur(@renderedInput, {relatedTarget: document.body}) expect(@renderedInput.value).toBe('') it 'should no longer have the `focused` class', -> ReactTestUtils.Simulate.focus(@renderedInput) expect(ReactTestUtils.scryRenderedDOMComponentsWithClass(@renderedField, 'focused').length).toBe(1) - ReactTestUtils.Simulate.blur(@renderedInput) + ReactTestUtils.Simulate.blur(@renderedInput, {relatedTarget: document.body}) expect(ReactTestUtils.scryRenderedDOMComponentsWithClass(@renderedField, 'focused').length).toBe(0) describe "when the user double-clicks a token", -> diff --git a/spec/fixtures/emails/email_17.html b/spec/fixtures/emails/email_17.html new file mode 100644 index 000000000..bb784b8d4 --- /dev/null +++ b/spec/fixtures/emails/email_17.html @@ -0,0 +1,140 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
 
Dear FOOBAR,
It is my sincere pleasure to inform you that you have been selected for = +membership in the Honor Society of the School of General Studies. The Socie= +ty was created in 1997 to celebrate the academic achievement of exceptional= + GS scholars. Only juniors or seniors with a grade point average of 3.8 or = +above who have completed at least 30 points at place are eligible for me= +mbership. The chief aim of the Honor Society is to cultivate interaction am= +ong students committed to intellectual discovery and the faculty who enjoy = +teaching them.
Please join us for the Induction Ceremony, with a reception to follow. +
+

Induction Ceremony
+ Honor Society
+
+ Reception to follow.

+
The favor of a reply is requested by Friday, J= +anuary 29 at 5 p.m. You are invited to bring one guest; business attire is = +requested.
I look forward to celebrating with you soon.

Best wishes,
+ +=3D"Dean
+ FOO J. BAR
+ Dean
+ University
+
+
+ N.B. A printed letter concerning your selection has been mailed to = +your local address.

 
+
+
+
+
+
+
+
+ + + + +
+ + + + +
+ This email was sent to FOOBAR.BAZ@place.edu +
+ why did I get this?    unsubscribe from this list&= +nbsp;   update subscription pref= +erences +
+ place +
+
+ =20 +
+
+
diff --git a/spec/fixtures/emails/email_17_stripped.html b/spec/fixtures/emails/email_17_stripped.html new file mode 100644 index 000000000..cc7a6aedc --- /dev/null +++ b/spec/fixtures/emails/email_17_stripped.html @@ -0,0 +1,94 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
 
Dear FOOBAR,
It is my sincere pleasure to inform you that you have been selected for = +membership in the Honor Society of the School of General Studies. The Socie= +ty was created in 1997 to celebrate the academic achievement of exceptional= + GS scholars. Only juniors or seniors with a grade point average of 3.8 or = +above who have completed at least 30 points at place are eligible for me= +mbership. The chief aim of the Honor Society is to cultivate interaction am= +ong students committed to intellectual discovery and the faculty who enjoy = +teaching them.
Please join us for the Induction Ceremony, with a reception to follow. +
+

Induction Ceremony
+ Honor Society
+
+ Reception to follow.

+
The favor of a reply is requested by Friday, J= +anuary 29 at 5 p.m. You are invited to bring one guest; business attire is = +requested.
I look forward to celebrating with you soon.

Best wishes,
+ =3D"Dean
+ FOO J. BAR
+ Dean
+ University
+
+
+ N.B. A printed letter concerning your selection has been mailed to = +your local address.

 
+
+
+
+
+
+
+
+ + + + +
+ + + + +
+ This email was sent to FOOBAR.BAZ@place.edu +
+ why did I get this?    unsubscribe from this list&= +nbsp;   update subscription pref= +erences +
+ place +
+
+ =20 +
+
+
+ \ No newline at end of file diff --git a/spec/quoted-html-transformer-spec.coffee b/spec/quoted-html-transformer-spec.coffee index 9a7f6dcee..d0df73eef 100644 --- a/spec/quoted-html-transformer-spec.coffee +++ b/spec/quoted-html-transformer-spec.coffee @@ -19,7 +19,7 @@ describe "QuotedHTMLTransformer", -> re = new RegExp(QuotedHTMLTransformer.annotationClass, 'g') html.match(re)?.length ? 0 - [1..16].forEach (n) -> + [1..17].forEach (n) -> it "properly parses email_#{n}", -> opts = keepIfWholeBodyIsQuote: true expect(removeQuotedHTML("email_#{n}.html", opts)).toEqual readFile("email_#{n}_stripped.html") @@ -268,6 +268,35 @@ describe "QuotedHTMLTransformer", -> after: """ """ + # Test 12: Make sure that a single quote inside of a bunch of other + # content is detected. We used to have a bug where we were only + # looking at the common ancestor of blockquotes (and if there's 1 then + # the ancestor is itself). We now look at the root document for + # trailing text. + tests.push + before: """ +
+ Yo + + + + +
AB
C
SAVE ME
EF
+ Yo +
+ """ + after: """ +
+ Yo + + + + +
AB
C
SAVE ME
EF
+ Yo +
+ """ + it 'works with these manual test cases', -> for {before, after} in tests opts = keepIfWholeBodyIsQuote: true @@ -318,7 +347,7 @@ describe "QuotedHTMLTransformer", -> # `QuotedHTMLTransformer` needs Electron booted up in order to work because # of the DOMParser. xit "Run this simple funciton to generate output files", -> - [16].forEach (n) -> + [17].forEach (n) -> newHTML = QuotedHTMLTransformer.removeQuotedHTML(readFile("email_#{n}.html")) outPath = path.resolve(__dirname, 'fixtures', 'emails', "email_#{n}_raw_stripped.html") fs.writeFileSync(outPath, newHTML) diff --git a/src/flux/stores/message-body-processor.coffee b/src/flux/stores/message-body-processor.coffee index a6669c14e..a615a4ba9 100644 --- a/src/flux/stores/message-body-processor.coffee +++ b/src/flux/stores/message-body-processor.coffee @@ -49,10 +49,15 @@ class MessageBodyProcessor # allow them to modify anything but the body for the time being. for extension in MessageStore.extensions() continue unless extension.formatMessageBody - virtual = message.clone() - virtual.body = body - extension.formatMessageBody({message: virtual}) - body = virtual.body + latestBody = body + try + virtual = message.clone() + virtual.body = body + extension.formatMessageBody({message: virtual}) + body = virtual.body + catch err + NylasEnv.emitError(err) + body = latestBody # Find inline images and give them a calculated CSS height based on # html width and height, when available. This means nothing changes size diff --git a/src/services/quoted-html-transformer.coffee b/src/services/quoted-html-transformer.coffee index 8d4dc2979..51503c18d 100644 --- a/src/services/quoted-html-transformer.coffee +++ b/src/services/quoted-html-transformer.coffee @@ -117,12 +117,7 @@ class QuotedHTMLTransformer # end of a message. If there were non quoted content after, it'd be # inline. - # We first need to find the common ancestor of the quoteElements. - # This tells us what depth to look at to determine if there's real - # content after us. - ancestor = DOMUtils.commonAncestor(quoteElements) - - trailingQuotes = @_findTrailingQuotes(ancestor, quoteElements) + trailingQuotes = @_findTrailingQuotes(doc, quoteElements) # Only keep the trailing quotes so we can delete them. quoteElements = _.intersection(quoteElements, trailingQuotes)