Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 30 additions & 0 deletions app/lib/email_processor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -395,6 +395,7 @@
fragment = Nokogiri::HTML5.fragment(html)
normalize_empty_html_blocks(fragment)
normalize_html_text_nodes(fragment)
normalize_html_block_edge_breaks(fragment)
trim_html_edge_breaks(fragment)
fragment.css('a[href]').each { |link| link['target'] = '_blank' }

Expand Down Expand Up @@ -582,6 +583,35 @@
node.remove
end

# Gmail can encode the same blank line either as an empty block between
# paragraphs or as a <br> at the edge of a content block. Move edge breaks
# between block siblings so both representations render identically.
def normalize_html_block_edge_breaks(fragment)
fragment.css('blockquote,div,li,p').each do |block|

Check notice on line 590 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L590

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)
normalize_html_block_edge_break(block, :previous)
normalize_html_block_edge_break(block, :next)
end
end

def normalize_html_block_edge_break(block, direction)
edge_child = direction == :previous ? first_content_child(block) : last_content_child(block)
return unless edge_child&.name == 'br'

Check notice on line 598 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L598

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)

sibling = nearest_non_whitespace_sibling(block, direction)
return unless sibling&.element? && (sibling.name == 'br' || html_block_node?(sibling))

Check notice on line 601 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L601

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)

loop do
edge_child = direction == :previous ? first_content_child(block) : last_content_child(block)
break unless edge_child&.name == 'br'

Check notice on line 605 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L605

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)

edge_child.remove
end
return if sibling.name == 'br'

Check notice on line 609 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L609

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)

separator = Nokogiri::XML::Node.new('br', block.document)

Check notice on line 611 in app/lib/email_processor.rb

View check run for this annotation

codefactor.io / CodeFactor

app/lib/email_processor.rb#L611

Prefer double-quoted strings unless you need single quotes to avoid extra backslashes for escaping. (Style/StringLiterals)
direction == :previous ? block.add_previous_sibling(separator) : block.add_next_sibling(separator)
end

def trim_html_edge_breaks(fragment)
([fragment] + fragment.css('blockquote,div,li,p').to_a).each do |parent|
first_content_child(parent)&.remove while first_content_child(parent)&.name == 'br'
Expand Down
29 changes: 29 additions & 0 deletions spec/features/entries_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,35 @@
expect(page).to have_content 'You need to login or sign up before continuing.'
end

it 'renders one blank line for every mixed Gmail paragraph separator' do
paid_user.entries.destroy_all
email = FactoryBot.build(
:email,
to: [{ token: paid_user.user_key, host: ENV['SMTP_DOMAIN'], email: "#{paid_user.user_key}@#{ENV['SMTP_DOMAIN']}"}],
body: "First paragraph\n\nSecond paragraph\n\nThird paragraph. Lorem ipsum\n\nFourth paragraph. Lorem ipsum",
vendor_specific: {
stripped_html: '<div>First paragraph<br></div><div>Second paragraph</div><div><br></div><div>Third paragraph.&nbsp;Lorem ipsum</div><div><br></div><div>Fourth paragraph.&nbsp;Lorem ipsum</div>'
}
)

EmailProcessor.new(email).process
processed_entry = paid_user.entries.reload.first

sign_in paid_user
visit day_entry_url(year: processed_entry.date.year, month: processed_entry.date.month, day: processed_entry.date.day)

rendered_entry = page.find('.s-scrollable')
expect(rendered_entry.all(:xpath, './*').map(&:tag_name)).to eq(%w[div br div br div br div])
expect(rendered_entry.text.split("\n")).to eq(
[
'First paragraph',
'Second paragraph',
'Third paragraph. Lorem ipsum',
'Fourth paragraph. Lorem ipsum'
]
)
end

it 'should show an entry stored at a non-midnight datetime' do
sign_in user
entry.update_columns(date: Time.utc(2026, 7, 17, 15, 30, 0), body: '<p>Afternoon journal entry</p>')
Expand Down
79 changes: 79 additions & 0 deletions spec/models/email_processor_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,24 @@
expect(paid_user.entries.reload.first.body).to eq("<div>Blah blah blah. Blah blah.</div><br><div>Blah blah blah. Blah!</div>")
end

it "stores consistent separators from mixed Gmail paragraph markup" do
paid_user.entries.destroy_all
email = FactoryBot.build(
:email,
to: [{ token: paid_user.user_key, host: ENV['SMTP_DOMAIN'], email: "#{paid_user.user_key}@#{ENV['SMTP_DOMAIN']}"}],
body: "First paragraph\n\nSecond paragraph\n\nThird paragraph. Lorem ipsum\n\nFourth paragraph. Lorem ipsum",
vendor_specific: {
stripped_html: '<div>First paragraph<br></div><div>Second paragraph</div><div><br></div><div>Third paragraph.&nbsp;Lorem ipsum</div><div><br></div><div>Fourth paragraph.&nbsp;Lorem ipsum</div>'
}
)

EmailProcessor.new(email).process

expect(paid_user.entries.reload.first.body).to eq(
'<div>First paragraph</div><br><div>Second paragraph</div><br><div>Third paragraph.&nbsp;Lorem ipsum</div><br><div>Fourth paragraph.&nbsp;Lorem ipsum</div>'
)
end

it "removes a trailing em-dash separator followed by a signature line" do
paid_user.entries.destroy_all
email = FactoryBot.build(
Expand Down Expand Up @@ -149,6 +167,67 @@ def clean(html)
expect(clean(html)).to eq('<div>First</div><br><div>Second</div>')
end

it 'normalizes mixed Gmail paragraph separators to one blank line' do
html = '<div>First paragraph<br></div><div>Second paragraph</div><div><br></div><div>Third paragraph.&nbsp;Lorem ipsum</div><div><br></div><div>Fourth paragraph.&nbsp;Lorem ipsum</div>'

expect(clean(html)).to eq('<div>First paragraph</div><br><div>Second paragraph</div><br><div>Third paragraph.&nbsp;Lorem ipsum</div><br><div>Fourth paragraph.&nbsp;Lorem ipsum</div>')
end

describe 'paragraph separator variants' do
{
'a trailing break in the preceding block' => [
'<div>First<br></div><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'a leading break in the following block' => [
'<div>First</div><div><br>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'an explicit break between blocks' => [
'<div>First</div><br><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'an empty div between blocks' => [
'<div>First</div><div><br></div><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'an NBSP-only paragraph between blocks' => [
'<div>First</div><p>&nbsp;</p><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'duplicate trailing and empty-block separators' => [
'<div>First<br></div><div><br></div><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'multiple trailing breaks' => [
'<div>First<br><br></div><div>Second</div>',
'<div>First</div><br><div>Second</div>'
],
'multiple consecutive empty blocks' => [
'<div>First</div><div><br></div><p>&nbsp;</p><div><br></div><div>Second</div>',
'<div>First</div><br><div>Second</div>'
]
}.each do |description, (html, expected)|
it "normalizes #{description} to exactly one blank line" do
expect(clean(html)).to eq(expected)
end
end

it 'normalizes separators inside a Gmail wrapper' do
html = '<div><div>First<br></div><div>Second</div><div><br></div><div>Third</div></div>'

expect(clean(html)).to eq('<div><div>First</div><br><div>Second</div><br><div>Third</div></div>')
end

it 'does not invent a blank line between adjacent blocks' do
expect(clean('<div>First</div><div>Second</div>')).to eq('<div>First</div><div>Second</div>')
end

it 'preserves a single authored line break within a paragraph' do
expect(clean('<div>First line<br>Second line</div>')).to eq('<div>First line<br>Second line</div>')
end
end

it 'removes leading and trailing empty blocks' do
html = '<p><br></p><div>Content</div><p>&nbsp;</p>'

Expand Down
Loading