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

Stop editing subject on Reply #221

Closed
ghost opened this Issue Jul 1, 2014 · 17 comments

Comments

5 participants
@ghost

ghost commented Jul 1, 2014

When I'm participating in an email conversation Rainloop likes to change the subject like so:

Re[2]: Original Subject Here

Please remove that [2] junk from the Subject

@rbeuque74

This comment has been minimized.

rbeuque74 commented Jul 1, 2014

Interested by knowing the reason of the [2]

@ghost

This comment has been minimized.

ghost commented Jul 1, 2014

I'm not sure why it happens.

strange

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 2, 2014

I think it is by design in this piece of code of admin.js:

/**
 * @param {string} sPrefix
 * @param {string} sSubject
 * @param {boolean=} bFixLongSubject = true
 * @return {string}
 */
Utils.replySubjectAdd = function (sPrefix, sSubject, bFixLongSubject)
{
    var
        oMatch = null,
        sResult = Utils.trim(sSubject)
    ;

    if (null !== (oMatch = (new window.RegExp('^' + sPrefix + '[\\s]?\\:(.*)$', 'gi')).exec(sSubject)) && !Utils.isUnd(oMatch[1]))
    {
        sResult = sPrefix + '[2]: ' + oMatch[1];
    }
    else if (null !== (oMatch = (new window.RegExp('^(' + sPrefix + '[\\s]?[\\[\\(]?)([\\d]+)([\\]\\)]?[\\s]?\\:.*)$', 'gi')).exec(sSubject)) &&
        !Utils.isUnd(oMatch[1]) && !Utils.isUnd(oMatch[2]) && !Utils.isUnd(oMatch[3]))
    {
        sResult = oMatch[1] + (Utils.pInt(oMatch[2]) + 1) + oMatch[3];
    }
    else
    {
        sResult = sPrefix + ': ' + sSubject;
    }

    sResult = sResult.replace(/[\s]+/g, ' ');
    sResult = (Utils.isUnd(bFixLongSubject) ? true : bFixLongSubject) ? Utils.fixLongSubject(sResult) : sResult;
//  sResult = sResult.replace(/^(Re|Fwd)[\s]?\[[\d]+\]:/ig, '$1:');
    return sResult;
};

The first if statement seems to match the subject of the email you are replying on. I can replicate the issue, but only if I reply to emails from mailing lists. They contain text like "[rainloop-webmail]". I'm looking at the code realy hard but I don't exactly understand what these 2 if statements are trying to achieve... I think it has to do with the text inside the square brackets.

Can anybody help to figure this out?

Ralf.

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 2, 2014

I tried removing the entire line

    if (null !== (oMatch = (new window.RegExp('^' + sPrefix + '[\\s]?\\:(.*)$', 'gi')).exec(sSubject)) && !Utils.isUnd(oMatch[1]))
    {
        sResult = sPrefix + '[2]: ' + oMatch[1];
    }

And changing the "else if" on the next line to just "if". But this does not seem to solve the problem. Perhaps the problem is not in the admin.js :)

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 2, 2014

Okay, the problem is not only there when replying to an email with brackets in the subject. But when there is already a "Re" in the subject. If there is a Re[2] in the subject, RainLoop changes it to a Re[3]... So this is a counter of some sort...

The above code is also in app.js. Removed it there and also changed this:

sSubject = sSubject.replace(/^Re(\[[\d]+\]|):[\s]{0,3}Re(\[[\d]+\]|):/gi, 'Re' + (0 < iCounter ? '[' + iCounter + ']' : '') + ':');

to this

sSubject = sSubject.replace(/^Re(\[[\d]+\]|):[\s]{0,3}Re(\[[\d]+\]|):/gi, 'Re' + ':');

But that also does not fix the problem.

@RainLoop, are we missing something here? When I edit the js files directly on my system (IE vi rainloop/v/1.6.7.133/static/js/app.js) it looks like it changes nothing in the behaviour of my RainLoop installation. I have set [cache] "enable = Off" and changed the index, but the code still seems cached somewhere?

@RainLoop

This comment has been minimized.

Owner

RainLoop commented Jul 2, 2014

  1. RainLoop Webmail uses app.min.js (compiled by #gulp)

  2. You need to change code in

dev/Common/Utils.js

or
https://github.com/RainLoop/rainloop-webmail/blob/master/dev/Common/Utils.js

  1. Then run #gulp to rebuild js files (app.js, app.min.js and others)

  2. If you want to remove reply counter, you just need to uncomment one line in code.

//  sResult = sResult.replace(/^(Re|Fwd)[\s]?\[[\d]+\]:/ig, '$1:');

Re[5]: xxx -> Re: xxx

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 3, 2014

Okay, it took me a while to learn (I'm no Git or coding guru...) how to clone this git repo onto my debian compiler box, but I succeeded. Modified Utils.js and then install NPM for gulp and all dependencies... Gulp ran clean (on the master repo, not my own fork...) and I copied index.php, data and rainloop folders to a test folder of my webserver.

All is looking okay when I reply. No more Re[2] etc.! And the Proxy patch also seems to work nice! 👍

But this is not a sustainable way of maintaining my RainLoop installation when new version come out etc. I would love to come up with a patch which introduces an option to enable or disable the reply counter, but that would take me a lightyear to figure out how to code that :(

@AndrewSav

This comment has been minimized.

Contributor

AndrewSav commented Jul 3, 2014

This is a dupe of #202

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 4, 2014

Not really. #202 is about not changing the subject at all. So the first reply does also not add a "Re:". This report is about the reply counter.

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 5, 2014

I created an option to enable or disable the reply counter and issued pull request #227.

@kaustavdm

This comment has been minimized.

kaustavdm commented Jul 10, 2014

Also line 450 of Utils.js needs to change from:

sResult = sPrefix + '[2]: ' + oMatch[1];

to

sResult = sPrefix + ': ' + oMatch[1];
@ervee

This comment has been minimized.

Contributor

ervee commented Jul 11, 2014

Are you sure? The RainLoop team itself said:
If you want to remove reply counter, you just need to uncomment one line in code.

// sResult = sResult.replace(/^(Re|Fwd)[\s]?\[[\d]+\]:/ig, '$1:');

@kaustavdm

This comment has been minimized.

kaustavdm commented Jul 11, 2014

@ervee Yes. This line stops the counter from incrementing. But, the line I talked about adds the counter if it does not exist. It explicitly adds [2] to the subject line. That is something you may need to modify in #227 as well

@kaustavdm

This comment has been minimized.

kaustavdm commented Jul 11, 2014

@ervee Btw, I tested it through Firefox's debugger, with breakpoints on all these lines. Can you test this through some debugger and re-confirm?

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 11, 2014

I did see the line that adds [2], but _if_ it is added, does the code sResult = sResult.replace(/^(Re|Fwd)[\s]?\[[\d]+\]:/ig, '$1:'); not remove it again?
I think if you remove the [2] from the code, it will break functionality for those who actually _want_ that behavior.

I just did some real-life tests on my code change, without a debugger, so I only see the end result, which looks okay to me.

I hope RainLoop will implement the Change / Pull Request. @RainLoop, can you comment on this?

@kaustavdm

This comment has been minimized.

kaustavdm commented Jul 11, 2014

@ervee If there is a need to show the count (though I still don't understand why this count is needed at all), it is better to add it only if requested through configuration. Adding it everytime and removing it if not requested is not a good architecture.

@ervee

This comment has been minimized.

Contributor

ervee commented Jul 11, 2014

I know, but this was the easiest way 😀 I'll take a look if I can change it without too much of a hassle.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment