View Issue Details
ID | Project | Category | View Status | Date Submitted | Last Update |
---|---|---|---|---|---|
0021083 | mantisbt | mentions | public | 2016-06-11 04:27 | 2016-07-09 19:28 |
Reporter | atrol | Assigned To | dregad | ||
Priority | normal | Severity | minor | Reproducibility | always |
Status | closed | Resolution | fixed | ||
Product Version | 1.3.0-rc.1 | ||||
Target Version | 1.3.0 | Fixed in Version | 1.3.0 | ||
Summary | 0021083: 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 | ||||
Tags | No tags attached. | ||||
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 |
|
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:
@vboctor, your thoughts would be appreciated. |
|
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. |
|
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 ? |
|
Work in progress PR https://github.com/mantisbt/mantisbt/pull/786 |
|
MantisBT: master-1.3.x 352d8f77 2016-06-12 01:14 Details Diff |
Tests: add case for issue 0021083 |
Affected Issues 0021083 |
|
mod - tests/Mantis/MentionParsingTest.php | Diff File | ||
MantisBT: master-1.3.x dc662052 2016-06-12 13:15 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 |
Affected Issues 0021083 |
|
mod - core/mention_api.php | Diff File | ||
MantisBT: master-1.3.x 166930eb 2016-07-03 01:55 Damien Regad Details Diff |
Fix 0021083: User @ mentions improvements PR https://github.com/mantisbt/mantisbt/pull/786 |
Affected Issues 0021083 |
|
mod - core/mention_api.php | Diff File | ||
mod - core/string_api.php | Diff File | ||
mod - tests/Mantis/MentionParsingTest.php | Diff File |