Comment #1 on issue 3754 by christop...@delphix.com: URLs in descriptions/comments no longer clickable links
https://code.google.com/p/reviewboard/issues/detail?id=3754

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 <christ...@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.

--
You received this message because this project is configured to send all issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

--
You received this message because you are subscribed to the Google Groups 
"reviewboard-issues" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to reviewboard-issues+unsubscr...@googlegroups.com.
To post to this group, send email to reviewboard-issues@googlegroups.com.
Visit this group at http://groups.google.com/group/reviewboard-issues.
For more options, visit https://groups.google.com/d/optout.

Reply via email to