3754: URLs in descriptions/comments no longer clickable links

christop*********@delph****** (Google Code) (Is this you? Claim this profile.)
July 14, 2015 
What version are you running?
2.0.12

What's the URL of the page containing the problem?
https://reviews.csiden.org/r/94/

What steps will reproduce the problem?
1. Include a URL in "Description", "Testing Done", or any review comment. (Example on above link is last line of "Testing Done" section.)

What is the expected output? What do you see instead?
The URL should be a clickable link.

What operating system are you using? What browser?
This definitely happens in Chrome and Firefox on Linux and Chrome on OSX. I  haven't tried any other browsers.

Please provide any additional information below.
The links were clickable in prior versions, they stopped working when we upgraded from 2.0.10 -> 2.0.12).
#1 christop*********@delph****** (Google Code) (Is this you? Claim this profile.) 
Looking at the code it seems like the links are supposed to be getting rendered by the RB.formatText function in reviewboard/static/rb/js/utils/textUtils.js:

RB.formatText = function($el, options) {
    options = options || {};

    if (options.richText) {
        if (options.newText !== undefined) {
            $el.html(options.newText)
        }

        $el
            .addClass('rich-text')
            .find('a')
                .attr('target', '_blank');

        RB.LinkifyUtils.linkifyChildren($el[0], options.bugTrackerURL);
    } else if (options.newText !== undefined) {
        $el
            .html(RB.LinkifyUtils.linkifyText(options.newText || '',
                                              options.bugTrackerURL,
                                              options.isHTMLEncoded))
            .removeClass('rich-text');
    }
};

But when tracing the page load it turns out neither condition is true (neither options.richText, nor options.newText is present). The caller is the formatText() function in reviewboard/static/rb/js/views/reviewRequestEditorView.js (it gets called once for description, testing done, etc. when the page loads), this function takes 'options' as input as well and then adds some defaults, but every time it is called the 'options' object is not present, the caller of formatText() on line 616 appears to have been changed in:

commit 9b6f5a7dca96d27d7296cada55bd5609c8b10943
Author: Christian Hammond <christian@beanbaginc.com>
Date:   Mon Oct 6 02:25:50 2014

    Switch exclusively to server-side Markdown rendering.

With the following diff:

@@ -559,9 +613,7 @@ RB.ReviewRequestEditorView = Backbone.View.extend({
              * though.
              */
             _.each(this.$('.field-text-area'), function(el) {
-                var $el = $(el);
-
-                this.formatText($el, $el.text());
+                this.formatText($(el));
             }, this);
 
             if (this.model.get('editable')) {

It used to pass in '$el.text()' as the second option to formatText() and that's what eventually got passed to the linkify function, now the second argument to formatText() is the options object and the linkification function is looking for a property called 'newData', which isn't there because the caller who has access to $el.text() isn't passing it in anymore. I was able to get links to be clickable again by changing the above code to be:

@@ -559,9 +613,7 @@ RB.ReviewRequestEditorView = Backbone.View.extend({
             * though.
             */
            _.each(this.$('.field-text-area'), function(el) {
                var $el = $(el);

                this.formatText($el, { newData: $el.text() });
             }, this);
 
             if (this.model.get('editable')) {

But I'm curious if that seems like a reasonable thing to do? For one thing it's not setting the options.richText property that the linkification code is looking for.
brennie
#2 brennie 
This bug was fixed on the release-2.0.x and master branches as (c371073eec3cc10e266ebcc15234d13edd14afb5). The fix will appear in Review Board 2.0.19 and 2.5.
  • +Fixed