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

Regression: conditional substitution [[ ]] is broken #192

Closed
macdeport opened this issue Jun 13, 2022 · 14 comments
Closed

Regression: conditional substitution [[ ]] is broken #192

macdeport opened this issue Jun 13, 2022 · 14 comments

Comments

@macdeport
Copy link

macdeport commented Jun 13, 2022

This appears between version 3.12pre30 and 3.12pre43 at least with %cc(name,bracketMail())%

@RealRaven2000
Copy link
Owner

Sorry can you be more detailed in your error description.

What exactly is the line you use, what does it do and what do you expect? Also do you use it on a reply email, forward or new? Conditionals that contain address parts cannot be used in "write new" case because all conditional removals have to happen when the editor loads. Also, what version of Thunderbird are you using - I am currently testing with Thunderbird 102.0b4 - the new ESR will be released at the end of this month!

@macdeport
Copy link
Author

macdeport commented Jun 14, 2022

Thunderbird 78.14.0

Here is my quote header in reply tab:

On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>
> To: %to(name,bracketMail())%[[
> Cc: %cc(name,bracketMail())%]]
> Subject: %subject%
> Message-ID: %Message-ID%
[[> Reply-to: %reply-to(bracketMail())%
]]>

You are welcome

@RealRaven2000
Copy link
Owner

Thunderbird 78.14.0

Here is my quote header in reply tab:

On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>
> To: %to(name,bracketMail())%[[
> Cc: %cc(name,bracketMail())%]]
> Subject: %subject%
> Message-ID: %Message-ID%
[[> Reply-to: %reply-to(bracketMail())%
]]>

You are welcome

I am just testing that how - this is what I get when I paste this quote header - I assume you have "use HTML" switched off:

image

It's actually better to export your settings (this will include quote header and your template) so I don't have to guess them:

image

There is probably a way to do what you want to do using the script tags: %{% and %}%, I will write some tests for that next.

@RealRaven2000
Copy link
Owner

I tried writing some test script using the JavaScript feature (which is documented here ) but unfortunately there is no safe way to include the variable "reply-to" (because JavaScript sees the "-" as a minus and then assumes there are 2 variables "reply" and "to") - it probably needs some deep patching to work. So I think with the current functions it may sadly not be possible to conditionally leave out the reply-to (if it is set by the original sender).

I will do some more tests...

@macdeport
Copy link
Author

It's actually better to export your settings (this will include quote header and your template) so I don't have to guess them

Of course: I have just sent to you the JSON configuration.

@macdeport
Copy link
Author

macdeport commented Jun 25, 2022

You have reproduced the wrong and annoying behaviour of conditional display of Cc:
image

@macdeport
Copy link
Author

When I reply to your mail Date: Tue, 14 Jun 2022 14:10:10 +0100 SmartTemplate 3.12.pre30 generates:

floating-screenshot-175232

while the last SmartTemplate 3.12.pre74 generates:

floating-screenshot-175058

@RealRaven2000
Copy link
Owner

while the last SmartTemplate 3.12.pre74 generates:

floating-screenshot-175058

At first I could not reproduce this with Thunderbird 91.10 and Thunderbird 102.0b8 - cc is left out if it is not set as a mail header.

Here is a test quote header is used myself:

<br> 
<style type="text/css"> 
#newHeaderBruxane { 
  background-color:rgba(210,210,210,0.15);
  border:1px rgb(210,210,210) solid;
  box-shadow: 4px 4px 1px rgba(50,50,50,0.15);
  border-radius:3px;
  display:inline-block;
  font-size: x-small;
  padding:0.8em;
} 
#newHeaderBruxane b {  
  color: #990033;
  display: inline-block; 
  font-weight:bold; 
  min-width: 5em; 
  max-width:none; 
}                
</style>
<div id="newHeaderBruxane">
<b>Betreff:</b> %subject% <br>
<b>An:</b> %to%  <br>
<b>Von:</b> %from%  <br>
[[<b>CC:</b> %cc(name)%  <br>]]
<b>Datum:   </b>%X:=sent%%A%, %dateshort% %H%:%M%
</div>

I believe it is the evil function "replace line breaks with <br>" there is something wrong. I think replacing [[ ]] parts only works if there is an explicit line break <br> in the line containing it.
I had thought about removing this antifeature for a long time, and just mandating the user to add <br> where he wants new lines. Can you try replacing your quote header with the following:

image

On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>
> To: %to(name,bracketMail())%[[<br>
> Cc: %cc(name,bracketMail())%]]<br>
> Subject: %subject%<br>
> Message-ID: %Message-ID%<br>
[[> Reply-to: %reply-to(bracketMail())%<br>
]]>

@RealRaven2000
Copy link
Owner

RealRaven2000 commented Jun 25, 2022

Another workaround: if you must use "Replace line breaks with <br>, leave out the empty parentheses after bracketMail:

On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>
> To: %to(name,bracketMail)%[[<br>
> Cc: %cc(name,bracketMail)%]]<br>
> Subject: %subject%<br>
> Message-ID: %Message-ID%<br>
[[> Reply-to: %reply-to(bracketMail)%<br>
]]>

the replacement algorithm has trouble with parsing out the nested (...())

Since you are not using an argument such as bracketMail(square) you don't even need the round brackets.
Another alternative is using bracketMail{} - I am replacing the inner round brackets with the curly brackets at some stage in the algorithm and I found it doesn't disrupt the function the simplifies and boils down address parameters to their fundamental function:

generalFunction = strInBrackets.replace(/%([\w-:=]+)(\([^)]+\))*%/gm, classifyReservedWord);

When it works: "> Cc: %cc(name,bracketMail{})%<br>" ==> "> Cc: <br>"
it can be safely removed by the simplify() function

@RealRaven2000
Copy link
Owner

The root of the problem is a replacement routine that for some reason only captures the last occurence of the bracketMail() pattern - here is an output from console:

msg
"<br>On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>> To: %to(name,bracketMail())%<br>[[> Cc: %cc(name,bracketMail())%<br>]]> Subject: %subject%<br>> Message-ID: %Message-ID%<br>[[> Reply-to: %reply-to(bracketMail())%<br>]]>"
msg.replaceAll(/%(.*)(bracketMail\(([^)]*))\)/g, "%$1bracketMail\{$3\}")
"<br>On %X:=sent%%dateformat(d/m/Y H:M:S)% %from(name,bracketMail())% wrote:<br>> To: %to(name,bracketMail())%<br>[[> Cc: %cc(name,bracketMail())%<br>]]> Subject: %subject%<br>> Message-ID: %Message-ID%<br>[[> Reply-to: %reply-to(bracketMail{})%<br>]]>" 

the second string only shows the last, "reply-to(bracketMail{})" processed. I did substitute the function replace() with replaceAll() with no success.

@RealRaven2000
Copy link
Owner

I found a better regular expression for finding the bracketMail() / bracketName() parts within a %function( )% parameter group that was less greedy than %(.*). Instead I am now using %([a-zA-Z][^)]+) which is a lot more specific. Basically a % function, must start with a letter and cannot include a closing ). This prevents including all following instances and breaks the matches up into 3 singles.

Test:

"<br>To: %to(name,bracketMail())%<br>[[> Cc: %cc(name,bracketMail())%<br>]]><br>[[> Reply-to: %reply-to(bracketMail())%<br>]]>"
.replaceAll(/%([a-zA-Z][^)]+)(bracketMail\(([^)]*))\)/gm, "%$1bracketMail\{$3\}")

"<br>To: %to(name,bracketMail{})%<br>[[> Cc: %cc(name,bracketMail{})%<br>]]><br>[[> Reply-to: %reply-to(bracketMail{})%<br>]]>"

Patched version:
smartTemplate-fx-3.12pre77.zip

@macdeport
Copy link
Author

macdeport commented Jun 25, 2022

#192 (comment)
Good workaround for 3.12.30 + except I have to edit all headers for every aliases ;-)

3.12.77 solve issue even with "odd" formed header :-) and Thunderbird 78.14.0 (above versions are apparently incompatible with my MacOS version 10.10.5 :-( )

@RealRaven2000
Copy link
Owner

#192 (comment) Good workaround for 3.12.30 + except I have to edit all headers for every aliases ;-)

3.12.77 solve issue even with "odd" formed header :-) and Thunderbird 78.14.0 (above versions are apparently incompatible with my MacOS version 10.10.5 :-( )

Nice one. I will have to eventually set minver higher than 78 (probably 91.0, there are a lot of changes that may become relevant) hopefully this release is still largely compatible with 78.

Technicality on Regex - I also found out on Matrix chat that there is a simple way of making the %something()% pattern non-greedy (lazy) by appending ?, like this:

(%.*?)bracketMail\((.*?)\)(.*?%)

looks a lot simpler than my %([a-zA-Z][^)]+)(bracketMail\(([^)]*))\) but I will need to test whether it is really safe(r) and better.

@RealRaven2000
Copy link
Owner

Fixed in 3.12.1 on 28/06/2022

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

No branches or pull requests

2 participants