237538 - Text munger effects depends on nesting order

Status: RESOLVED FIXED

(Other Applications :: ChatZilla, defect)

Description

[James Ross]

In short, _foo |bar| baz_ works, but |foo _bar_ baz| misses the tele-type
effect. This is, I think, because the munger applies the rules in the order it
finds, and thus matches _/_ before |/|. Fixing will not be that pretty, but
could be done in the following way:
  - apply each regex or lambdaMatch function in turn, and keep track of the one
with the lowest index within the text.
  - run the lambdaReplace or "simple munger" code for the munger found with the
lowest start index.

Comment 1

[James Ross]

Attachment: Existing WIP patch

Comment 2

[:Gijs (out for now; he/him)]

Attachment: Full Patch, moving cz's mungers to new mungers.js file in xul/content/

This patch is ridiculously large and such. I'll spend a moment to try and move everything back so that at least the patch is smaller. I don't want that other version checked in, but it should be easier to see what the ... James and I did in that one.

Comment 3

[:Gijs (out for now; he/him)]

Attachment: [not-for-checkin] patch -wpu6 without the code move.

Comment 4

[:Gijs (out for now; he/him)]

Attachment: [not-for-checkin] patch -pu6 without the code move

OK, this should do I hope.

Comment 5

[:Gijs (out for now; he/him)]

self-nit-note-for-checkin: remove the semicolons after the helper functions at the end of munger.js

Comment 6

[Samuel Sieb]

Comment on attachment 244672 [details] [diff] [review]
Full Patch, moving cz's mungers to new mungers.js file in xul/content/

>+function CMungerEntry(name, regex, className, enable, startPriority, tagName)

This doesn't match how it's called.

> CMunger.prototype.addRule =
>+function mng_addrule(name, regex, className, priority, startPriority, enable)
[...]
>+    var entry = new CMungerEntry(name, regex, className, priority,
>+                                 startPriority, enable);

This is how it's called.

>+    if (!(priority in this.entries))
>+        this.entries.length = Math.max(this.entries.length, priority);

Why is this necessary?

> CMunger.prototype.munge =
>+function mng_munge(text, containerTag, data)
[...]
>+        for (var i = this.entries.length; i >= 0; i--)
>         {
>+            if (i in this.entries)

Why not just |for (var i in this.entries)| like you used elsewhere?

>+            // There is no need to munger the "before" text, as we should

munger -> munge

Comment 7

[:Gijs (out for now; he/him)]

Attachment: Patch with ssieb's nits, less <wbr> and more colours

Re: ssieb's review:
- Corrected the method signature mistake. Oops.
- Removed the array length extension, which was useless.
- Left the loop code as-is, because looping with for ... in would be in the wrong order (which is important there) and it would also take along other things done to the array variable (I'm not sure if it'd list "length" but I seem to remember it would. Which would break the code.
- Corrected the spelling/grammar oopsie.

Additional stuff:
- Added CSS and code to make all kinds of links retain mIRC colour/style markup.
- Added some checks so the munger wouldn't call insertText on empty strings (adding useless empty spans and <wbr/>s)
- Took along fixes for the .mailto and bugzilla mungers from the dep bugs. This patch applies to trunk again. :-)

Comment 8

[:Gijs (out for now; he/him)]

Attachment: Working patch

Ugh. I was too happy copy-pasting stuff in that last patch. This one actually works. I also fixed some more classes in the CSS and removed the strike-through one because we don't use it at all.

Comment 9

[:Gijs (out for now; he/him)]

Nit: delete data.isUnder in mircResetColor should be delete data.isUnderline (see http://lxr.mozilla.org/seamonkey/source/extensions/irc/xul/content/static.js#1229 for comparison).

Comment 10

[:Gijs (out for now; he/him)]

Checking in mozilla/extensions/irc/jar.mn;
new revision: 1.31; previous revision: 1.30
Checking in mozilla/extensions/irc/xul/content/commands.js;
new revision: 1.119; previous revision: 1.118
Checking in mozilla/extensions/irc/xul/content/handlers.js;
new revision: 1.146; previous revision: 1.145

RCS file: /cvsroot/mozilla/extensions/irc/xul/content/mungers.js,v
Checking in mozilla/extensions/irc/xul/content/mungers.js;
initial revision: 1.1

Checking in mozilla/extensions/irc/xul/content/output-base.css;
new revision: 1.14; previous revision: 1.13
Checking in mozilla/extensions/irc/xul/content/output-window.js;
new revision: 1.18; previous revision: 1.17
Checking in mozilla/extensions/irc/xul/content/scripts.xul;
new revision: 1.13; previous revision: 1.12
Checking in mozilla/extensions/irc/xul/content/static.js;
new revision: 1.230; previous revision: 1.229
Checking in mozilla/extensions/irc/xul/lib/munger.js;
new revision: 1.24; previous revision: 1.23

--> FIXED