Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Setting map_hyphen=True in routes.py breaks codemirror editor #769

Closed
amerikan opened this issue Feb 3, 2015 · 17 comments
Closed

Setting map_hyphen=True in routes.py breaks codemirror editor #769

amerikan opened this issue Feb 3, 2015 · 17 comments

Comments

@amerikan
Copy link
Contributor

amerikan commented Feb 3, 2015

The codemirror breaks once I set the map_hypen to True.

In my routes.py I have:

routers = dict(
    BASE=dict(
        default_application='mycoolapp',
        map_hyphen=True,
    )
)

When I try to use the web-based web2py editor it fails to load.

Looking at the console I see the following 404's:

screen shot 2015-02-02 at 5 35 16 pm

The path goes from being an _2.9.5 to -2.9.5 which is expected behavior for the URLS, however because the directory starts with a _ and gets changed to - it causes problem. For example http://localhost:8000/admin/static/**-2.9.5**/js/typeahead.min.js

@amerikan amerikan changed the title Setting map_hyphen=False in routes breaks codemirror editor Setting map_hyphen=False in routes.py breaks codemirror editor Feb 3, 2015
@amerikan
Copy link
Contributor Author

amerikan commented Feb 3, 2015

To add additional info to this issue, I originally tested it on version 2.9.5, but just now tested it on version 2.9.12 and issue still exists.

To reproduce it's quite easy. Download 2.9.12. Then, to the root of the web2py directory add routes.py and it place the following:

routers = dict(

    # base router
    BASE=dict(
        default_application='welcome',
        map_hyphen=True,
    ),
)

Now in the admin interface, click on Reload routes. Now, try to opening and editing any file using the built-in web2py code editor (uses codemirror).

@amerikan amerikan changed the title Setting map_hyphen=False in routes.py breaks codemirror editor Setting map_hyphen=True in routes.py breaks codemirror editor Feb 3, 2015
@niphlod
Copy link
Member

niphlod commented Feb 3, 2015

uhm. I'm not convinced this is the right fix.... and in fact I can't reproduce the original issue.......
my "_2.9.5 to -2.9.5" doesn't take place in 2.9.12. The problem is already fixed by c0536d3 . Can you please doublecheck ?

@amerikan
Copy link
Contributor Author

amerikan commented Feb 3, 2015

@niphlod Hi, I tested it on a fresh copy of 2.9.12. I don't think it has much to do with commit you're referencing. This is a separate issue and goes back to at least 2.9.5. It has nothing to do with allowing map_hyphen to work for application names as the commit suggest.

More system details:
OSX 10.10.2
Python 2.7.6
Web2py 2.9.12

Are you reloading the routes after creating the routes.py with the above code in the root dir?

@niphlod
Copy link
Member

niphlod commented Feb 4, 2015

yep, I am. Are you ? your problem is with urls in static/ but the code ALREADY skips static . Can you make a test that doesn't involve an app (i.e. from the shell)?

@amerikan
Copy link
Contributor Author

amerikan commented Feb 4, 2015

@niphlod I now feel 100% sure this is a bug. I'm sorry you can't reproduce it, maybe you're not doing something right or I'm not being clear.

I tried to further debug this. I placed a print statement to see what the value of self.controller would be.

screen shot 2015-02-04 at 9 35 30 am

I then re-started the web2py and refreshed the editor page and checked the terminal and it printed the following values as self.controllers:

default
default
default
default
mercurial
default
default
debug
static
static
static
static
static
static
static
static
default
static
static
static
static
default
static/_2.9.12  <---- look
static/_2.9.12  <---- look
static/_2.9.12  <---- look
default
default
default
default
default
default
default
default
static/_2.9.12  <---- look
static/_2.9.12  <---- look
default
default
static

Note that the output contains a couple static/_2.9.12 as controllers.

Now check the current code:

if self.map_hyphen:
    self.controller = self.controller.replace('_', '-') # <--- does not care if static or not, conversion for controller will still take effect, and since `static/_2.9.12` is a "controller" it will get replaced by `static/-2.9.12`
    if self.controller != 'static' and not self.controller.startswith('static/'): # <--- since static check is here
        self.application = self.application.replace('_', '-')
        self.function = self.function.replace('_', '-')

In my PR I placed it nested in the static/ check and this has fixed the issue for me:

if self.map_hyphen:
    if self.controller != 'static' and not self.controller.startswith('static/'):
        self.application = self.application.replace('_', '-') 
        self.controller = self.controller.replace('_', '-') # <- will not get touched since it's inside static/ check
        self.function = self.function.replace('_', '-')

Let me know if you have more questions. Would like to get the fix merged into master :shipit:

@niphlod
Copy link
Member

niphlod commented Feb 4, 2015

if indeed this is happening, the bug is static/something being a controller, not the rewrite logic...

@niphlod
Copy link
Member

niphlod commented Feb 4, 2015

imho the error is the way admin is coded.

{{cm=URL('static%s' % (response.static_version and '/_' + response.static_version  or ''),'codemirror')}}
<link rel="stylesheet" href="{{=cm}}/lib/codemirror.css">

this doesn't pass through rewrite logic as it should (calling URL('somethingwithaslashinside') is wrong ) and so the controller passed - in some weird occasion - is 'static/_1.2.3' and everything is screwed up.
Let's patch admin correctly instead of altering the rewrite logic that is perfect.

@niphlod
Copy link
Member

niphlod commented Feb 4, 2015

moreover, I found out that there is a subtle bug handling response.static_version and response.static_version_urls ........... stay tuned for a PR that:

  • adds response.static_version_urls to admin (so code is decluttered and will ALWAYS work)
  • fixes the subtle bug
  • cleanups admin
    if you trust me, you can go ahead and close this (as already said, the rewrite logic is perfect, it's the use inside admin of URL() that is inherently wrong)... or wait for a PR and test that one.

@mdipierro : please keep this on hold a bit

@niphlod niphlod closed this as completed in 1b34216 Feb 6, 2015
mdipierro added a commit that referenced this issue Feb 6, 2015
@cowbert
Copy link

cowbert commented Mar 3, 2015

this does not seem to fix the issue with admin. If I set static_version_urls = False in admin/models/0.py, admin is still appending _ + static_version for its static asset URLs. I believe the line 498 in gluon/globals.py should be:

#if self.static_version and not self.static_version_urls
if self.static_version and self.static_version_urls:

@niphlod
Copy link
Member

niphlod commented Mar 3, 2015

did you fetch the latest admin code ?

@amerikan
Copy link
Contributor Author

amerikan commented Mar 3, 2015

I just pulled from master and it seems to be ok so far...

@cowbert
Copy link

cowbert commented Mar 4, 2015

I just downloaded master.zip from github and admin still wants to read from admin/static/_2.9.12/ (regardless of what static_version_urls is set to in 0.py; here are the results when static_version_urls=False) :

[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/js/bootstrap.min.js HTTP/1.1" 200 28631
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/_2.9.12/plugin_statebutton/js/bootstrap-switch.js HTTP/1.1" 404 267
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/images/questions.png HTTP/1.1" 200 5036
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/_2.9.12/plugin_statebutton/css/bootstrap-switch.css HTTP/1.1" 404 269
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/_2.9.12/css/bootstrap.min.css HTTP/1.1" 404 247
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/_2.9.12/css/bootstrap_essentials.css HTTP/1.1" 404 254
[04/Mar/2015:02:26:27 -0500] "GET /web2py/admin/static/_2.9.12/css/bootstrap-responsive.min.css HTTP/1.1" 404 258

I have a custom routes.py, but that shouldn't be interfering, since it is literally copied from the examples:

BASE = '/web2py'
routes_in = (
    # do not reroute admin unless you want to disable it
    (BASE + '/admin', '/admin/default/index'),
    (BASE + '/admin/$anything', '/admin/$anything'),
    # do not reroute appadmin unless you want to disable it
    (BASE + '/$app/appadmin', '/$app/appadmin/index'),
    (BASE + '/$app/appadmin/$anything', '/$app/appadmin/$anything'),
    # do not reroute static files
    (BASE + '/$app/static/$anything', '/$app/static/$anything'),
    # reroute favicon and robots, use exable for lack of better choice
    ('/favicon.ico', '/examples/static/favicon.ico'),
    ('/robots.txt', '/examples/static/robots.txt'),
    # do other stuff
    #((r'.*http://otherdomain\.com.* (?P<any>.*)', r'/app/ctr\g<any>')),
    # remove the BASE prefix
   (BASE + '/$anything', '/$anything'),
)
routes_out = [(x, y) for (y, x) in routes_in]

@niphlod
Copy link
Member

niphlod commented Mar 4, 2015

what's the problem with that ? what resources are not loaded in your case ?

@cowbert
Copy link

cowbert commented Mar 4, 2015

The problem is that with static_version_urls=False, admin should not be using static version URLs when Apache is configured with AliasMatch to map ([^/]+)/static/(.*) to applications/$1/static/$2

@niphlod
Copy link
Member

niphlod commented Mar 4, 2015

ok, the issue you're mentioning comes from 3 set of distinct set of problems:

  • an aliasmatch to ([^/]+)/static/(.*) is wrong if you want to support static versioning, and I don't see why you should keep it that way. It's 3 years old.
  • static_version_urls works to turn ANY URL('static', 'something') into the "versioned" uri, in addition to the ones generated by response.files
  • if you don't remove static_version, you'll still get files generated with response.files with the "versioned" uri, independently from the static_version_urls

@cowbert
Copy link

cowbert commented Mar 4, 2015

What I'm saying is I don't want static versioning at all, but there is no way to disable it (in admin) correctly, because gluon/globals.py still adds it, even if static_version_urls= False

@niphlod
Copy link
Member

niphlod commented Mar 4, 2015

response.static_version = None

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants