View Issue Details

IDProjectCategoryView StatusLast Update
0021083mantisbtmentionspublic2016-07-09 19:28
ReporteratrolAssigned Todregad 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Product Version1.3.0-rc.1 
Target Version1.3.0Fixed in Version1.3.0 
Summary0021083: Link to user page is not always displayed when using @ mentioning
Description

Seems to happen if the user name is at the end of a line and another line is following

@atrol
a

TagsNo tags attached.

Relationships

related to 0020837 closedvboctor Support @ mentions 

Activities

dregad

dregad

2016-06-12 05:45

developer   ~0053328

Last edited: 2016-06-12 09:52

View 2 revisions

The problem seems to be that the mentions processing takes place after text formatting. Bugnotes being multiline fields, the MantisCoreFormatting plugin performs an nl2br conversion, so the (\r)\n are converted to HTML line breaks.

So "@atrol\r\na" becomes "@atrol< br />\r\na", which gets parsed as "@atrol<br" by the mentions tokenizer and is not a valid username.

IMO it would be both more accurate and more efficient if the tokenizer were refactored to use regular expressions, since we already define what is a valid username in configuration ($g_user_login_valid_regex).

I'll submit a PR later.

EDIT: replace ' by " to avoid quoted mentions to be considered as email addresses

dregad

dregad

2016-06-12 09:51

developer   ~0053329

Funny that (apostrophe)(at)username gets parsed as an e-mail address, I never realized that ' was a valid char in this context.

That being said, I believe the current default user valid login pattern is too permissive to allow my idea of using regex without changing it: usernames can contain spaces, dots and even @ signs.

For '.' and '@', the idea was to allow use of email addresses as usernames, which makes sense; I don't know why we allow space though, that does not make any sense to me.

Alternative approaches:

  • change the default validation pattern (remove space)
  • use a specific (hardcoded ?) pattern for mentions that will only match a subset of all valid usernames

@vboctor, your thoughts would be appreciated.

vboctor

vboctor

2016-06-12 10:37

manager   ~0053330

To answer your question, I agree that we should limit usernames to characters that makes sense. However, it is unclear to me how to do that in a way that enables users with affected usernames to move to the new valid patterns. I suggest we open an issue to track this and have the discussion there.

I'm OK with @ mentions working only with usernames that matches what you think make sense. i.e. use a more strict pattern and it becomes one of the motivations for people to move to the desired state for usernames.

As for using a regex rather than a tokenizer, I think I started with this approach then moved away from it. Don't recall the reasons specifically. But it was about looking for tokens with appropriate delimiters about them that rather than matches of a regex.

I recall there was an issue when you have two users with common prefix: e.g. vboctor and vboctor123. You don't want the latter to match as the former or both. Not sure if this is working in my current implementation or not.

dregad

dregad

2016-06-12 12:06

developer   ~0053332

After a bit of research, the problem with the regex approach is that some of the scenarios require lookahead and/or lookbehind to properly match (or not) - that makes for a somewhat complex regex (more than I initially thought).

Some preliminary testing results https://regex101.com/r/lQ0lL8/1 - feedback appreciated.

Also see my comment [1] about test case '@vboctor%%%%%' - why is it expected to fail ?

[1] https://github.com/mantisbt/mantisbt/commit/a9a20393d0ed765a72d69f0f68e536458748b894#commitcomment-17835719

dregad

dregad

2016-06-12 18:57

developer   ~0053345

Work in progress PR https://github.com/mantisbt/mantisbt/pull/786

Related Changesets

MantisBT: master-1.3.x 352d8f77

2016-06-12 05:14:00

dregad

Details Diff
Tests: add case for issue 0021083
mod - tests/Mantis/MentionParsingTest.php Diff File

MantisBT: master-1.3.x dc662052

2016-06-12 17:15:42

Damien Regad

Details Diff
Use specific regex for parsing mentions

It turns out that the current $g_user_login_valid_regex pattern
cannot be used as it allows spaces. Furthermore, special handling is
required to process e-mail address-like strings.

For this reason, a custom regex is built for the purpose of mentions
parsing, supporting only a subset of the allowed usernames.

Fixes 0021083
mod - core/mention_api.php Diff File

MantisBT: master-1.3.x 166930eb

2016-07-03 05:55:10

Damien Regad

Details Diff
Fix 0021083: User @ mentions improvements

PR https://github.com/mantisbt/mantisbt/pull/786
mod - core/mention_api.php Diff File
mod - core/string_api.php Diff File
mod - tests/Mantis/MentionParsingTest.php Diff File

Issue History

Date Modified Username Field Change
2016-06-11 04:27 atrol New Issue
2016-06-11 04:28 atrol Relationship added related to 0020837
2016-06-12 05:45 dregad Assigned To => dregad
2016-06-12 05:45 dregad Status new => assigned
2016-06-12 05:45 dregad Note Added: 0053328
2016-06-12 05:45 dregad Target Version => 1.3.0
2016-06-12 09:51 dregad Note Added: 0053329
2016-06-12 09:52 dregad Note Edited: 0053328 View Revisions
2016-06-12 10:37 vboctor Note Added: 0053330
2016-06-12 12:06 dregad Note Added: 0053332
2016-06-12 18:57 dregad Note Added: 0053345
2016-06-12 20:17 vboctor Category bugtracker => mentions
2016-07-03 06:14 dregad Changeset attached => MantisBT master-1.3.x 352d8f77
2016-07-03 06:14 dregad Changeset attached => MantisBT master-1.3.x dc662052
2016-07-03 06:14 dregad Changeset attached => MantisBT master-1.3.x 166930eb
2016-07-03 06:14 dregad Status assigned => resolved
2016-07-03 06:14 dregad Resolution open => fixed
2016-07-03 06:14 dregad Fixed in Version => 1.3.0
2016-07-09 19:28 vboctor Status resolved => closed