Closed Bug 850675 Opened 7 years ago Closed 7 years ago

editing the splinter url always appends a /, which results in a broken url

Categories

(bugzilla.mozilla.org :: Splinter, defect, critical)

Production
x86
macOS
defect
Not set
critical

Tracking

()

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

Details

Attachments

(1 file)

editing the splinter url always appends a /, which results in a broken url
Assignee: nobody → glob
caused by bug 298268:

> # Stop complaining if the URL has no trailing slash.
> # XXX - This hack can go away once bug 303662 is implemented.
> if ($name =~ /(?<!webdot)base$/) {
>     $value = "$value/" if ($value && $value !~ m#/$#);
> }
Attached patch patch v1Splinter Review
this patch adds a single method which all the code uses to generate the urls.

i updated get_review_url to remove the trailing /, and also made it always return absolute urls (to fix email-embedded urls).
Attachment #724478 - Flags: review?(dkl)
Comment on attachment 724478 [details] [diff] [review]
patch v1

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

Nice. r=dkl with the two things I commented about.

::: extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl
@@ +1,4 @@
>  [% IF flag && flag.status == '?' && flag.type.name == 'review' && attachment && attachment.ispatch %]
>  
> +Review
> +[%+ Bugzilla.splinter_review_url(request.bug_id, request.attach_id) FILTER none %]

Need to [% USE Bugzilla %] here for this to work. [% USE Bugzilla %] comes after the 'after_summary' hook in template/en/default/request/email.txt.tmpl. Or you can move the line up in the core template but don't really need to if you add it in the hook template.

::: extensions/Splinter/template/en/default/pages/splinter.html.tmpl
@@ -114,5 @@
>  <![endif]-->
>  
>  <div id="helpful-links">
> -  <a id="allReviewsLink" href="[% urlbase FILTER none %][% Param('splinter_base') FILTER js %]">
> -    [reviews]</a>

Why are you removing this? Useful for seeing a list of past reviews and isn't broken AFAIK.
Attachment #724478 - Flags: review?(dkl) → review+
(In reply to David Lawrence [:dkl] from comment #3)
> Why are you removing this? Useful for seeing a list of past reviews and
> isn't broken AFAIK.

ah, i never got any reviews during my testing, but i never published a review under that environment.  i'll restore that link and fix it.
Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified .htaccess
modified extensions/Splinter/Extension.pm
modified extensions/Splinter/lib/Util.pm
modified extensions/Splinter/template/en/default/admin/params/splinter.html.tmpl
modified extensions/Splinter/template/en/default/hook/attachment/edit-action.html.tmpl
modified extensions/Splinter/template/en/default/hook/attachment/list-action.html.tmpl
modified extensions/Splinter/template/en/default/hook/request/email-after_summary.txt.tmpl
modified extensions/Splinter/template/en/default/hook/request/queue-after_column.html.tmpl
modified extensions/Splinter/template/en/default/pages/splinter.html.tmpl
Committed revision 8665.
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.