Modify

Opened 5 years ago

Closed 5 years ago

Last modified 5 years ago

#4912 closed defect (fixed)

Doesn't work

Reported by: jason.trahan@… Owned by: richard
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 5 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 5 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 5 years ago by richard

  • Resolution set to fixed
  • Status changed from new to closed

[5514]

  • fixed.

comment:4 follow-up: Changed 5 years ago by jason.trahan@…

  • Resolution fixed deleted
  • Status changed from closed to reopened

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 5 years ago by richard

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 follow-up: Changed 5 years ago by 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

comment:7 Changed 5 years ago by bobbysmith007

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 5 years ago by bobbysmith007

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 5 years ago by richard

  • Resolution set to fixed
  • Status changed from reopened to closed

(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 5 years ago by richard

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.

Add Comment

Modify Ticket

Action
as closed .
The resolution will be deleted. Next status will be 'reopened'.
Author


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

 
Note: See TracTickets for help on using tickets.