Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Strip gmail's html quote properly #49

Merged

Conversation

ValdikSS
Copy link
Contributor

Gmail web interface uses not-totally-standard quotes: div (or blockquote) with class=gmail_quote, instead of blockquote with type=cite.

Without this fix, NestedQuote Remover can't clean anything in gmail correspondence. Without "generic div hander" cite depth is computed incorrectly, so don't remove it (it works for depth=1 but not for others).

Gmail web interface uses not-totally-standard quotes: div (or blockquote) with class=gmail_quote, instead of blockquote with type=cite.

Without this fix, NestedQuote Remover can't clean anything in gmail correspondence. Without "generic div hander" cite depth is computed incorrectly, so don't remove it (it works for depth=1 but not for others).
if (depth > maxDepth) {
node.remove();
} else {
processHtmlBlockquotes(node.childNodes, replyHeaderHandling, maxDepth, depth + 1);
}
}

else if (node.nodeType === Node.ELEMENT_NODE &&
node.nodeName === "DIV" &&
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it possible that the "gmail variant" has a "BLOCKQUOTE" or "DIV" as the node name???
If gmail only uses DIV... then why do we need the altered if condition above?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it is possible when you already have a long conversation with multiple quotations from different clients (gmail web/regular different email clients).
This patch actually adds gmail web quote handling and fixes support of other clients' quotes.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've simplified the whole section and pushed it to the dev-branch.
Please let me know if this still works for you.
Thank you.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new version doesn't work for this exact email with maximum quote depth allowed > 1.
Try it yourself.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Commit a16a219 doesn't work correctly on this email (with maximum quote depth allowed = 3 it strips down to 1 quotation), and ce5808b does not work at all with quote depth allowed > 1.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ValdikSS I'm sorry... but I hope you understand that I'm going to let this issue rest for a while.
I really tried to reproduce the issue... but couldn't.
Your modifications to the code would introduce complexity and also may lead to unwanted side-effects (since at some point you treat simple DIVs as quotes, etc.).
Hope you understand that I'm a bit hesitant here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I really tried to reproduce the issue... but couldn't.

That's because you're using "reply" button while in a plain text view. It does add color bars, just like on your screenshot (no non-colored bars). In this mode the extension works correctly. However, it doesn't while replying in HTML mode.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've emailed you the video. Please watch it.

Oh. Okay... just watched it.
That was helpful. 😄
I'll get back to it. (Probably next weekend.)
Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello, any updates?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No... sorry.
I've opened another issue to work on this.
Haven't found the time for it so far. 😞

processHtmlBlockquotes(node.childNodes, replyHeaderHandling, maxDepth, depth);
}

else if (node.nodeType === Node.ELEMENT_NODE &&
Copy link
Owner

@4ch1m 4ch1m Sep 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... yet another variant.
Can you provide the HTML-source of a Gmail quote for me? So I can have a look at it?
Thank you very much!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your email server rejects .eml files.
Re- обход блокировки.zip
Password is in email.

@4ch1m 4ch1m merged commit 110fa63 into 4ch1m:dev_nestedquoteremover-me Sep 15, 2021
}

else if (node.nodeType === Node.ELEMENT_NODE &&
node.nodeName === "DIV") // generic div
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ValdikSS : I'm not sure that this last else-if is a bit too generic.
Basically this would mean, that ANY node that is a DIV will get treated as a blockquote.
Which is not good.
I tend to remove this last else block, but keep the other ones.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Check my email example, it won't work correctly without this code. If you can make it better, go ahead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants