Closed Bug 955189 Opened 10 years ago Closed 10 years ago

Nicks starting or ending by a non-alphanumeric character aren't detected correctly

Categories

(Instantbird Graveyard :: Conversation, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: aleth)

References

Details

Attachments

(3 files, 1 obsolete file)

*** Original post on bio 1757 at 2012-11-04 22:04:00 UTC ***

*** Due to BzAPI limitations, the initial description is in comment 1 ***
Blocks: 954993
Attached image Screenshot of the issue
*** Original post on bio 1757 as attmnt 2045 at 2012-11-04 22:04:00 UTC ***

We identified at least 2 cases that cause Show Nick to miss nicks:
- nicks ending in "-" (there's no word boundary between "-" and " " so the regexp doesn't match)
- nicks beginning and ending by "_", they are underlined before the nick detection.
*** Original post on bio 1757 at 2012-11-04 22:10:46 UTC ***

Probably similar issues arise in detecting these nicks for pings (if the user has such a nick).
*** Original post on bio 1757 at 2013-02-06 10:12:05 UTC ***

Another nick that isn't detected: ^MrJiM
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1757 as attmnt 2465 at 2013-05-30 16:34:00 UTC ***

This replaces \b(nick)\b with \W(nick)\W. Apart from fixing the issues above, this would actually match unicode nicks (if there are protocols which support these).

I could not find a JS library which implements unicode word boundary matching, which would cause less false positives for non-latinate languages.
Attachment #8354232 - Flags: review?(florian)
*** Original post on bio 1757 at 2013-05-30 16:36:12 UTC ***

How hard would it be to add tests for this?
Attached patch PatchSplinter Review
*** Original post on bio 1757 as attmnt 2466 at 2013-05-30 18:07:00 UTC ***

Adds some more comments and gets rid of the capturing parentheses (the captured matches were not used and they cost performance).
Attachment #8354233 - Flags: review?(florian)
Comment on attachment 8354232 [details] [diff] [review]
Patch

*** Original change on bio 1757 attmnt 2465 at 2013-05-30 18:07:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354232 - Attachment is obsolete: true
Attachment #8354232 - Flags: review?(florian)
*** Original post on bio 1757 at 2013-05-30 18:07:51 UTC ***

Forgot to mention I also got rid of the global flag.
Assignee: nobody → aleth
Status: NEW → ASSIGNED
*** Original post on bio 1757 as attmnt 2467 at 2013-05-30 19:26:00 UTC ***

This changes \b to \W in the ping-detection regex too for consistency.
Attachment #8354234 - Flags: review?(florian)
Comment on attachment 8354233 [details] [diff] [review]
Patch

*** Original change on bio 1757 attmnt 2466 at 2013-05-30 22:37:59 UTC ***

This version is more convincing than attachment 8354232 [details] [diff] [review] (bio-attmnt 2465) :-). Thanks for looking into this annoying issue!
Attachment #8354233 - Flags: review?(florian) → review+
Comment on attachment 8354234 [details] [diff] [review]
Analogue patch for ping detection

*** Original change on bio 1757 attmnt 2467 at 2013-05-30 22:38:24 UTC ***

Let's be consistent in our nick matching! :)
Attachment #8354234 - Flags: review?(florian) → review+
*** Original post on bio 1757 at 2013-05-30 22:44:48 UTC ***

Editing the summary to reflect what we are actually fixing here.

The second case mentionned in comment 0 is not fixed (and is IMHO a completely different issue, that we should file separately).
Summary: Some nicks aren't detected by Show Nick → Nicks starting or ending by a non-alphanumeric character aren't detected correctly
*** Original post on bio 1757 at 2013-05-31 00:38:21 UTC ***

Fixed in http://hg.instantbird.org/instantbird/rev/3748f7c417fd, we need to file a follow up for comment 0 part 2.
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.5
*** Original post on bio 1757 at 2013-05-31 00:39:49 UTC ***

This had a second check-in: http://hg.instantbird.org/instantbird/rev/43559dca3293
You need to log in before you can comment on or make changes to this bug.