Modify

Opened 15 years ago

Closed 14 years ago

Last modified 14 years ago

#4912 closed defect (fixed)

Doesn't work

Reported by: Jason Trahan Owned by: Richard Liao
Priority: normal Component: TracTweakUiPlugin
Severity: major Keywords:
Cc: Trac Release: 0.11

Description

So I followed the instructions as followed.

  1. Created new URL Reference of /newticket
  1. Added editcc script with the following script
$(document).ready(function() {
	// edit cc
	alert("Testing");
});
  1. Clicked on new ticket and it would never fire off the alert.

When I dug into this more I noticed that the script reference was coming out as


<script src="/tractweakui/tweakui_js/%2Fnewticket.js" type="text/javascript"></script>


however when I attempt to go that script Trac says it does not exist.

I have tested this in both firefox and IE

Attachments (0)

Change History (10)

comment:1 Changed 15 years ago by jason.trahan@…

So I think I found the problem. When it gets stored in the database the url path is /newticket. However when it gets rendered to the browser it comes out as %2Fnewticket. because of this when it looks in the database it is not properly matching up the links.

comment:2 Changed 15 years ago by Jason Trahan

OK Found the problem. In process_question you have the code as followed

        if req.path_info.startswith(tweakui_js_path):
            path_pattern = req.path_info[len(tweakui_js_path) + 1 : -3]
            js_files = TracTweakUIModel.get_path_scripts(self.env, path_pattern)



It should be setup like this.

        if req.path_info.startswith(tweakui_js_path):
            path_pattern = urllib.unquote(req.path_info[len(tweakui_js_path) + 1 : -3])
            js_files = TracTweakUIModel.get_path_scripts(self.env, path_pattern)



Once this is in place then it starts working right.

comment:3 Changed 15 years ago by Richard Liao

Resolution: fixed
Status: newclosed

[5514]

  • fixed.

comment:4 Changed 15 years ago by Jason Trahan

Resolution: fixed
Status: closedreopened

One more change needed to be made to make it work right. Also in web_ui.py look for the line.


src= req.base_path + "/tractweakui/tweakui_js/" + urllib.quote(path_pattern, "") + ".js")()


and change it to


src= req.base_path + "/tractweakui/tweakui_js/" + urllib.quote(path_pattern, "/") + ".js")()


comment:5 in reply to:  4 Changed 15 years ago by Richard Liao

Replying to jason.trahan@twtelecom.com:

One more change needed to be made to make it work right. Also in web_ui.py look for the line.


src= req.base_path + "/tractweakui/tweakui_js/" + urllib.quote(path_pattern, "") + ".js")()


and change it to


src= req.base_path + "/tractweakui/tweakui_js/" + urllib.quote(path_pattern, "/") + ".js")()


The purpose of urllib.quote(path_pattern, safe = "") is that we normally treat as a normal js file, like: '%2Fab%2Fcd.js'. If we use this urllib.quote(path_pattern, safe = "/") instead, this js file will look like this: '/ab/cd.js'. Also, in my test, when using safe = "", the result is ok.

>>> urllib.unquote(urllib.quote("/ab/cd.js", ""))
'/ab/cd.js'
>>> urllib.unquote(urllib.quote("/ab/cd.js", "/"))
'/ab/cd.js'

comment:6 Changed 14 years ago by Russ Tyndall

I can confirm that this plugin, in its current state, does not work for me.

When I set the url regex to newticket it does not try to include the javascript at all.

When I try to set the url to /newticket it inserts the correct script tags, but doesn't actually serve the javascript (404 errors).

Both of these seem like errors

comment:7 Changed 14 years ago by Russ Tyndall

The solution to newticket not matching was to change re.match to re.search in web_ui.py

  • tractweakui/web_ui.py

     
    203203        path_patterns = TracTweakUIModel.get_path_patterns(self.env)
    204204        # try to match pattern
    205205        for path_pattern in path_patterns:
    206             if re.match(path_pattern, req.path_info):
     206            if re.search(path_pattern, req.path_info):
    207207                break
    208208        else:
    209209            return stream

With this patch in place I can successfully use the plugin. This doesnt solve the above mentioned "/" problem, but its one step closer. The reason is because search searches the whole string while match on checks for a match at the beginning (implicit start anchor).

http://docs.python.org/library/re.html#re.search

HTH, Russ

comment:8 Changed 14 years ago by Russ Tyndall

Whew!, I spent rather more time on this plugin this evening than expected :)

I tried to look into why "/newticket" was failing but trac doesnt even seem to be trying to handle requests to urls like .../tractweakui/tweakui_js/%2Fnewticket.js (I dont get log messages from match_request, which I do if the extra %2F is not in the url).

As such it seems like perhaps embedding the regex that matches the url as part of a url *might* not be the best idea. Is there any reason not to just output the javascript directly into the script tag rather than outputting a script tag and automatically handling that request?

The patch below does just that and seems to work for me. Alternatively I would suggest giving each pattern a unique ID (which seems to be happening in the database) and then look up the javascript to serve based on ID rather than pattern (which can contain characters which screw up urls.)

  • tractweakui/web_ui.py

     
    2222
    2323from pkg_resources import resource_filename
    2424from genshi.filters.transform import Transformer
     25from trac.util import Markup
    2526
    2627import sys, os
    2728import re
     
    211212        filter_names = TracTweakUIModel.get_path_filters(self.env, path_pattern)
    212213        for filter_name in filter_names:
    213214            stream = self._apply_filter(req, stream, path_pattern, filter_name)
     215        js_files = TracTweakUIModel.get_path_scripts(self.env, path_pattern)
     216        if js_files:
     217            script = ";\n".join(js_files)
     218        else:
     219            script = ""
     220
    214221        stream = stream | Transformer('head').append(
    215             tag.script(type="text/javascript",
    216             src= req.base_path + "/tractweakui/tweakui_js/" + urllib.quote(path_pattern, "") + ".js")()
    217             )
     222            # Markup allows us to output javascript without it being weirdly escaped (which can mess up JQuery
     223            tag.script(Markup(script), type="text/javascript")())
    218224
    219225        return stream
    220226
    221227    # IRequestHandler methods
    222228
    223229    def match_request(self, req):
     230        return False # Dont handle dynamic requests right now
    224231        return req.path_info.startswith('/tractweakui/tweakui_js')
    225232
    226233    def process_request(self, req):
     
    245253        if not os.path.exists(filter_path):
    246254            return stream
    247255
    248         css_files = self._find_filter_files(filter_path, "css")
    249         js_files = self._find_filter_files(filter_path, "js")
     256        css_files = self._find_filter_files(filter_path, ".css")
     257        js_files = self._find_filter_files(filter_path, ".js")
    250258
    251259        for css_file in css_files:
    252260            stream = self._add_css(req, stream, filter_name, css_file)

Thanks for the work you put in on this plugin, I hope this patch helps get it to a more stable usable point.

Cheers, Russ

comment:9 Changed 14 years ago by Richard Liao

Resolution: fixed
Status: reopenedclosed

(In [7413]) fixed #4912, output the javascript directly into the script tag rather than in a script tag.

comment:10 in reply to:  6 Changed 14 years ago by Richard Liao

Replying to bobbysmith007:

I can confirm that this plugin, in its current state, does not work for me.

When I set the url regex to newticket it does not try to include the javascript at all.

When I try to set the url to /newticket it inserts the correct script tags, but doesn't actually serve the javascript (404 errors).

Both of these seem like errors

This seems occur publishing by apache. tracd or lighttd works as expected. I am not sure if apache do some automatic URL convertion.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Richard Liao.
The resolution will be deleted. Next status will be 'reopened'.

Add Comment


E-mail address and name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.