Closed
Bug 105865
Opened 23 years ago
Closed 10 years ago
bugzilla should pay attention to linebreaks when making bugnumbers to links
Categories
(Bugzilla :: Creating/Changing Bugs, defect)
Tracking
()
RESOLVED
FIXED
Bugzilla 5.0
People
(Reporter: duncan, Assigned: LpSolit)
References
Details
Attachments
(1 file, 2 obsolete files)
3.64 KB,
patch
|
dkl
:
review+
|
Details | Diff | Splinter Review |
Bugzilla ignores linebreaks when making bugnumbers to links. Found this is e.g. in bug 103843 example: 1. read this bug 2. think about this bug 3. comment this bug nothing of the example above should be a link was unable to find a duplicate, hope it is none :)
Comment 1•23 years ago
|
||
This was a conscious design decision. It'll always get some wrong, bug it'll get far more wrong (due to the forced line wrapping) if we switch it back to ignoring line breaks.
Status: NEW → RESOLVED
Closed: 23 years ago
Resolution: --- → WONTFIX
Comment 2•23 years ago
|
||
Which is another good reason we shouldn't be forcing line breaks.
Comment 3•20 years ago
|
||
*** Bug 251229 has been marked as a duplicate of this bug. ***
Comment 4•19 years ago
|
||
*** Bug 307658 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 6•19 years ago
|
||
*** Bug 140099 has been marked as a duplicate of this bug. ***
Assignee | ||
Comment 7•18 years ago
|
||
*** Bug 353333 has been marked as a duplicate of this bug. ***
Comment 10•17 years ago
|
||
You know, this would actually be fixable now that we do server-side wrapping. That is, unless a comment is already_wrapped, if we do quoteUrls before wrap_comment, we could ignore linebreaks.
OS: Other → All
Hardware: PC → All
Assignee | ||
Comment 11•17 years ago
|
||
OK, let's do that. And if it's easy to do, let's do it for 3.0.x as well.
Comment 12•17 years ago
|
||
I'm not sure how easy it will be, but we can see.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
Updated•17 years ago
|
Assignee: myk → create-and-change
Status: REOPENED → NEW
QA Contact: mattyt-bugzilla → default-qa
Comment 13•17 years ago
|
||
We'll tentatively set this to be targeted at 3.0, and then if we find it's too complex, we'll retarget it to 3.2.
Target Milestone: --- → Bugzilla 3.0
Assignee | ||
Updated•16 years ago
|
Target Milestone: Bugzilla 3.0 → Bugzilla 3.2
Assignee | ||
Updated•16 years ago
|
Severity: trivial → normal
Assignee | ||
Comment 14•16 years ago
|
||
I tested it with both already wrapped and not already wrapped comments, and it's working as expected. In order to ignore newlines for not yet already wrapped comments, you have to use \h instead of \s.
Assignee: create-and-change → LpSolit
Status: NEW → ASSIGNED
Attachment #343976 -
Flags: review?(mkanat)
Assignee | ||
Comment 15•16 years ago
|
||
(In reply to comment #14) > In order to ignore newlines for not yet already wrapped comments err... I meant exactly the opposite. You want to take newlines into account for not yet wrapped comments. Sorry for the confusion.
Comment 16•16 years ago
|
||
Comment on attachment 343976 [details] [diff] [review] patch, v1 >+ my $bug_re = qr/\Q$bug_word\E$s*\#?$s*(\d+)/i; You have $ in there twice? Does that even do anything? >+ my $comment_re = qr/comment$s*\#?$s*(\d+)/i; Same there... >+ $text =~ s~\b($bug_re(?:$s*,?$s*$comment_re)?|$comment_re) And there...does $s have some special meaning? I don't see it in the perlre or perlvar manpage.
Attachment #343976 -
Flags: review?(mkanat) → review-
Comment 17•16 years ago
|
||
Also, look at comment 0 in this bug. It's already fixed, it looks like. None of those things are links. Let's try with a newer comment: 1. read this bug 2. think about this bug 3. comment this bug
Severity: normal → minor
Target Milestone: Bugzilla 3.2 → Bugzilla 3.4
Comment 18•16 years ago
|
||
Yeah, it looks like quoteUrls is already working properly, based on my above comment.
Assignee | ||
Comment 19•16 years ago
|
||
Comment on attachment 343976 [details] [diff] [review] patch, v1 Please read carefuly. I define $s earlier in the code.
Attachment #343976 -
Flags: review- → review?(mkanat)
Assignee | ||
Comment 20•16 years ago
|
||
And no, this is not fixed upstream. I tested.
Comment 21•16 years ago
|
||
(In reply to comment #20) > And no, this is not fixed upstream. I tested. Then why is it working in comment 0 and comment 17 here?
Assignee | ||
Comment 22•16 years ago
|
||
(In reply to comment #21) > Then why is it working in comment 0 and comment 17 here? No idea. But I can definitely reproduce locally on tip and 3.2 as well as on landfill, see http://landfill.bugzilla.org/bugzilla-tip/show_bug.cgi?id=2456#c1
Assignee | ||
Comment 23•16 years ago
|
||
Oh, forget it. I know why. bug 1 and bug 2 do not exist on b.m.o, so they are not linkified.
Assignee | ||
Comment 24•16 years ago
|
||
Step to reproduce: bug 105864: read this bug 105865: think about this bug 105866: comment this bug. ;)
Updated•16 years ago
|
Attachment #343976 -
Flags: review?(mkanat) → review+
Comment 25•16 years ago
|
||
Comment on attachment 343976 [details] [diff] [review] patch, v1 Ahh, okay. :-) In that case, this looks fine. I assume that you've tested it and all. Also, as a side note, I think we might need a Bugzilla::Comment object that we can call methods on, like wrap and quote_urls, or something. But that's not super-important right now.
Comment 26•16 years ago
|
||
Changing things about those crazy regexes is something that I only want to do on tip, so I'd like to limit this to there.
Flags: approval+
Assignee | ||
Comment 27•16 years ago
|
||
(In reply to comment #26) > Changing things about those crazy regexes is something that I only want to do > on tip, so I'd like to limit this to there. Sure, no problem.
Assignee | ||
Comment 28•16 years ago
|
||
Checking in Bugzilla/Template.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v <-- Template.pm new revision: 1.92; previous revision: 1.91 done Checking in template/en/default/bug/comments.html.tmpl; /cvsroot/mozilla/webtools/bugzilla/template/en/default/bug/comments.html.tmpl,v <-- comments.html.tmpl new revision: 1.38; previous revision: 1.37 done
Status: ASSIGNED → RESOLVED
Closed: 23 years ago → 16 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 29•16 years ago
|
||
Perl 5.8.x doesn't understand \h. We have to replace it by [[:blank:]]. Seems to work fine now, and still behaves correctly with Perl 5.10.
Attachment #344009 -
Flags: review?(mkanat)
Updated•16 years ago
|
Attachment #344009 -
Flags: review?(mkanat) → review+
Assignee | ||
Comment 30•16 years ago
|
||
\h replaced by [[:blank:]] to make Perl 5.8 happy: Checking in Bugzilla/Template.pm; /cvsroot/mozilla/webtools/bugzilla/Bugzilla/Template.pm,v <-- Template.pm new revision: 1.93; previous revision: 1.92 done
Assignee | ||
Comment 31•14 years ago
|
||
Reopening! The patch is broken, see bug 514703 (which will back out the patch).
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Target Milestone: Bugzilla 3.4 → ---
Assignee | ||
Updated•14 years ago
|
Flags: approval+
Updated•14 years ago
|
Whiteboard: [blocker will fix]
Assignee | ||
Comment 32•14 years ago
|
||
I cannot do anything about it till bug 551468 is fixed, and then this bug shouldn't be a problem anymore.
Assignee: LpSolit → create-and-change
Comment 33•14 years ago
|
||
Okay, on trunk should we re-implement this patch, now that we're not doing server-side wrapping anymore? already_wrapped comments might still be a problem, though.
Comment 35•13 years ago
|
||
Bug 551468 was marked RESOLVED FIXED 2010-10-26 15:49:51 PDT, so in reference to Comment 33 I think this patch should be re-implemented.
Comment 38•11 years ago
|
||
As I couldn't find this bug: adding autolinkification in a comment
Comment 40•11 years ago
|
||
Frédéric, Glob: Reading comment 32 and comment 33 : Maybe [blocker will fix] is not true anymore ?
Assignee | ||
Comment 41•11 years ago
|
||
(In reply to bigstijn from comment #40) > Maybe [blocker will fix] is not true anymore ? Right, the problem still exists.
Assignee | ||
Comment 42•11 years ago
|
||
Now that bug 551468 has been fixed, we can now fix this bug correctly. I had to remove /o from regexps, else if a bug contained both already wrapped and non already wrapped comments, then the regexps weren't recompiled.
Assignee: create-and-change → LpSolit
Attachment #343976 -
Attachment is obsolete: true
Attachment #344009 -
Attachment is obsolete: true
Status: REOPENED → ASSIGNED
Attachment #779155 -
Flags: review?(dkl)
Assignee | ||
Updated•11 years ago
|
Target Milestone: --- → Bugzilla 5.0
Comment 43•11 years ago
|
||
(In reply to Frédéric Buclin from comment #41) > (In reply to bigstijn from comment #40) > > Maybe [blocker will fix] is not true anymore ? > > Right, the problem still exists. > > Blocks bug 514703 > No longer depends on bug 514703 > Whiteboard: remove [blocker will fix] Is the change from "depends" to "blocks" intentional? Bug 514703 was FIXED in January 2010.
Assignee | ||
Comment 44•11 years ago
|
||
Yes, that's intentional.
Comment 45•11 years ago
|
||
Comment on attachment 779155 [details] [diff] [review] patch, v2 Review of attachment 779155 [details] [diff] [review]: ----------------------------------------------------------------- Works well for me. r=dkl
Attachment #779155 -
Flags: review?(dkl) → review+
Assignee | ||
Updated•11 years ago
|
Flags: approval?
Updated•11 years ago
|
Flags: approval? → approval+
Assignee | ||
Comment 46•11 years ago
|
||
Committing to: bzr+ssh://lpsolit%40gmail.com@bzr.mozilla.org/bugzilla/trunk/ modified Bugzilla/Template.pm Committed revision 8700.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago → 11 years ago
Resolution: --- → FIXED
Comment 48•10 years ago
|
||
Mozilla/5.0 (X11; Linux x86_64; rv:27.0) Gecko/20100101 Firefox/27.0 SeaMonkey/2.24a1 ID:20130919003001 c-c:1ea1fc3586db m-c:803189f35921 BMO Production (Bugzilla 4.2.7+) This is supposed to have been FIXED on 14 August, but I still see "links" in comment #24. What happens if I copy it? Step to reproduce: bug 105864: read this bug 105865: think about this bug 105866: comment this bug. ;)
Comment 49•10 years ago
|
||
...and I see them in comment #48 too.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Assignee | ||
Comment 50•10 years ago
|
||
(In reply to Tony Mechelynck [:tonymec] from comment #48) > BMO Production (Bugzilla 4.2.7+) > > This is supposed to have been FIXED on 14 August, but I still see "links" in > comment #24. Target milestone says "5.0". BMO still runs 4.2.7.
Status: REOPENED → RESOLVED
Closed: 11 years ago → 10 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•