If you think a bug might affect users in the 57 release, please set the correct tracking and status flags for Release Management.

markdown isn't linkifying urls

RESOLVED FIXED in Bugzilla 6.0

Status

()

Bugzilla
Bugzilla-General
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: glob, Assigned: Albert Ting)

Tracking

({regression})

unspecified
Bugzilla 6.0
regression
Bug Flags:
approval +
blocking5.0 -

Details

Attachments

(1 attachment, 3 obsolete attachments)

(Reporter)

Description

3 years ago
markdown isn't linkifying all urls.

eg.

http://www.example.com
foo@example.com

currently http://www.example.com isn't converted to a link when markdown is enabled, however it is when markdown is disabled.

Updated

3 years ago
Depends on: 330707
Keywords: regression

Comment 1

3 years ago
That's because according to Markdown specification and its original implementation, you need to surround URLs with angle brackets. [1] But, that should be linkified anyway. Also, if we follow Markdown standards, we need to decide whether to omit angle brackets after linkifiying or leave them unchanged. Bugzilla does not remove them but standard Markdown does.

[1]: http://daringfireball.net/projects/markdown/syntax#autolink

Comment 2

3 years ago
Created attachment 8488277 [details] [diff] [review]
patch-v1.0

This might be a nasty hack!
Attachment #8488277 - Flags: review?(glob)
(Reporter)

Comment 3

3 years ago
Comment on attachment 8488277 [details] [diff] [review]
patch-v1.0

Review of attachment 8488277 [details] [diff] [review]:
-----------------------------------------------------------------

this needs to operate the same as quoteUrls (ie. honour SAFE_PROTOCOLS).
Attachment #8488277 - Flags: review?(glob) → review-

Comment 4

3 years ago
Created attachment 8488990 [details] [diff] [review]
patch-v2
Assignee: general → koosha.khajeh
Attachment #8488277 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #8488990 - Flags: review?(glob)

Updated

3 years ago
Attachment #8488990 - Flags: review?(glob)

Updated

3 years ago
Attachment #8488990 - Attachment is obsolete: true

Updated

3 years ago
Whiteboard: [roadmap: 5.0]

Comment 5

3 years ago
Created attachment 8490826 [details] [diff] [review]
patch-v2
Attachment #8490826 - Flags: review?(glob)
(Reporter)

Comment 6

3 years ago
Comment on attachment 8490826 [details] [diff] [review]
patch-v2

Review of attachment 8490826 [details] [diff] [review]:
-----------------------------------------------------------------

this is much better, however it doesn't cope well with links within quotes.

> "http://example.com"
comes out as
> "http://example.com&quot

the comment for _DoAutoLinks will also need updating.
Attachment #8490826 - Flags: review?(glob) → review-
Hi Koosha. Just looking at some of the remaining unresolved issues that is blocking us from branching for 5.0. This bug is one and was wondering if you will be able to take a look at this soon. I don't mind taking it if you are not going to be able able to. 

Thanks
dkl
Flags: needinfo?(koosha.khajeh)

Comment 8

3 years ago
(In reply to David Lawrence [:dkl] from comment #7)
> Hi Koosha. Just looking at some of the remaining unresolved issues that is
> blocking us from branching for 5.0. This bug is one and was wondering if you
> will be able to take a look at this soon. I don't mind taking it if you are
> not going to be able able to. 
> 
> Thanks
> dkl

OK, you can take it. But, take care not to double-linkify link definitions in reference-style links while you patch this bug.
Assignee: koosha.khajeh → general
Status: ASSIGNED → NEW
Flags: needinfo?(koosha.khajeh)

Updated

3 years ago
Severity: normal → blocker

Updated

3 years ago
Severity: blocker → normal
Flags: blocking5.0?
(Reporter)

Updated

3 years ago
Flags: blocking5.0? → blocking5.0+

Updated

3 years ago
Flags: blocking5.0+ → blocking5.0-
Target Milestone: Bugzilla 5.0 → Bugzilla 6.0
(Assignee)

Comment 9

3 years ago
Created attachment 8579747 [details] [diff] [review]
alternative v1

Attached is an alternative solution where I handle it in DoAnchors.

The only situation this patch is not able to handle is something like below, where a ';' appears just before the url.  This is because I need to do a negative lookbehind to ignore both &lt; and ['"<>].  Perhaps somebody can take this further.  

  ;http://mozilla.org
(Assignee)

Updated

3 years ago
Attachment #8579747 - Flags: review?(dkl)
Comment on attachment 8579747 [details] [diff] [review]
alternative v1

Review of attachment 8579747 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl

::: Bugzilla/Markdown.pm~
@@ +258,5 @@
>  
> +    # Last, handle "naked" references
> +    # Caveat, does not handle ;http://amazon.com
> +    $text =~ s{
> +        (                    

remove whitespace
Attachment #8579747 - Flags: review?(dkl) → review+

Updated

3 years ago
Flags: approval?
(Reporter)

Updated

3 years ago
Attachment #8490826 - Attachment is obsolete: true
(Reporter)

Updated

3 years ago
Assignee: general → altlist
Flags: approval? → approval+
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   1fbcede..1fa1aa5  master -> master
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.