cssUrlRewriting does not take context path into account

[GoogleCodeExporter]

When using CssUrlRewriting the result is sometimes wrong when the application 
is running with a context path different than /.

An example is an application running with /herpderp, with a css entry 
"background: url(../bitmaps/backdrop-side.jpg)"
In the file webapp/herp/css/file.css. 
Now this is rewritten to 
background: url("/herp/bitmaps/backdrop-side.jpg"), but the correct entry is 
background: url("/herpderp/herp/bitmaps/backdrop-side.jpg").

Original issue reported on code.google.com by marvin.l...@gmail.com on 27 Mar 2012 at 2:54

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 27 Mar 2012 at 8:45

• Changed state: Accepted
• Added labels: Milestone-Release-1.4.5

[GoogleCodeExporter]

Do you reproduce this problem when using wro4j maven plugin or WroFilter?

Original comment by alex.obj...@gmail.com on 27 Mar 2012 at 9:21

[GoogleCodeExporter]

I have only tried with the filter. 

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 5:06

[GoogleCodeExporter]

I am not sure whether the solution is adding contextpath or change the way the 
rewrite happens.
I have struggled with upgrading WRO for some days. In the previous version we 
used, 1.2.2, we had this css as output: 
background: url("../derp/css/../bitmaps/backdrop-side.jpg")
Now we get background: url("/derp/bitmaps/backdrop-side.jpg")
where it should be url("../derp/bitmaps/backdrop-side.jpg")
I have not seen what has changed in the rewrite, but it seems it removes to 
much.

So I guess it is not that context path is not taken into account, but that the 
leading .. is removed.

If I have the time, I may fork and make a solution proposal.

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 6:45

[GoogleCodeExporter]

Could you provide the version number you are using? Is it latest stable (1.4.4)?

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 6:56

[GoogleCodeExporter]

Yes.

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 9:49

[GoogleCodeExporter]

Well, this is strange and/or embarrassing. 
Suddenly I have no problems with the urls.
I was investigating aggregatedFolderPath and suddenly I could not reproduce the 
problem.
I am certain that I previously cleaned both browser and all other caches. 

I am certain seeing aggregatedFolderPath being null yesterday, causing the urls 
previously mentioned, bet now aggregatedFolderPath is "..".

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 12:56

[GoogleCodeExporter]

I also wasn't able to reproduce the problem yesterday. Strange....
The aggregatedFolderPath should never be null. It is important to reproduce the 
problem in order to fix it. 
Please, let me know if you can reproduce the problem.

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 1:00

• Changed state: New

[GoogleCodeExporter]

Btw, do you configure reloadCache or reloadModel to something else than 0? I 
suspect that this can be related to the scheduler used to reload the cache.

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 1:01

[GoogleCodeExporter]

reloadCache is set to 10 I think. I always restarted the server after doing 
changes.

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 1:05

[GoogleCodeExporter]

Can you check if it is possible to reproduce the problem after the reloadCache 
is triggered?

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 1:08

[GoogleCodeExporter]

When ReloadCacheRunnable is run the context used in 
CssUrlRewritingProcessor.replaceImageUrl(...) is null.

In WroManager.initAggregatedFolderPath(), 
Context.get().setAggregatedFolderPath(aggregatedFolder) is run, with 
aggregatedFolder set to "/wro-oa/".
The object returned by Context.get() in WroManager is different from the 
context injected into CssUrlRewritingProcessor.
But the only difference (based on the output from toString) is 
aggregatedFolderPath, being null in the use used in CssUrlRewritingProcessor.

Original comment by marvin.l...@gmail.com on 28 Mar 2012 at 1:40

[GoogleCodeExporter]

That confirms my assumption. At least we know that there is no problem when 
reloadCache  & reloadModel are set to 0.

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 1:46

[GoogleCodeExporter]

Original comment by alex.obj...@gmail.com on 28 Mar 2012 at 1:48

• Changed state: Accepted
• Removed labels: Milestone-Release-1.4.5

[GoogleCodeExporter]

In release 1.4.6 the code which handles the reloadCache & reloadModel updates 
was changed. As a consequence, this issue is fixed. 

The short description of the change is this:
- When reloadCache (or reloadModel) is greater than 0 (seconds), than after 
each X seconds the cache is flushed only. Previous versions also triggered the 
processing in a separate thread.

Original comment by alex.obj...@gmail.com on 10 May 2012 at 7:22

• Changed state: Fixed
• Added labels: Milestone-Release-1.4.6

[GoogleCodeExporter]

I have the same problem but the other way around. It works fine with 1.4.4 
(reloadCache  and reloadModel are set to 0) but it sometimes breaks with 1.4.6. 
Sometimes wro4j creates the correct urls 
url(../resources/images/windup_key.png) but sometimes wrong ones 
url(/resources/images/windup_key.png)

I tried to create a simple unit test but it always works in a small project 
setup. The only way to demonstrate the problem is so far my e4ds-template 
sample application.

Here the steps:
1. Download the zip from: 
https://github.com/ralscha/e4ds-template/zipball/wro4jtest 
2. mvn clean tomcat7:run or mvn tomcat7:run inside project directory
3. Clear Browser Cache
4. Open URL http://localhost:8080/e4
5. Check if top left corner shows a logo. 
6. Open http://localhost:8080/e4/wro/login.css?v=1.0.0  
   sometimes image url is: url(/resources/images/windup_key.png)
   and sometimes it's: url(../resources/images/windup_key.png)

It looks like that it works most of the time if the first request
goes to http://localhost:8080/e4/login.html. Maybe it has something to do
with the redirects that are happening when the start url is 
http://localhost:8080/e4

Same behaviour on Linux and Windows. When I debug the code then I see that 
sometimes aggregatedFolderPath is null. In that cases the url is wrong. 

Tested on Linux Mint 12 LXDE
OpenJdk IcedTea7 2.0 (7-b147-2.0-0ubuntu0.11.10.1)
Firefox 13.0

Windows 7
java version "1.7.0_05"
Java(TM) SE Runtime Environment (build 1.7.0_05-b05)
Java HotSpot(TM) 64-Bit Server VM (build 23.1-b03, mixed mode)
Chrome 19.0.1084.56m

Original comment by ralphschaer@gmail.com on 21 Jun 2012 at 5:14

[GoogleCodeExporter]

I've managed to start your application, but get a consistent behavior when 
using 1.4.6. Could you add a breakpoint to 
Wromanager#initAggregatedFolderPath() and tell me what happens during 
aggregatedFolderPath computation when the problem is reproducible?

Thanks,
Alex

Original comment by alex.obj...@gmail.com on 21 Jun 2012 at 9:08

[GoogleCodeExporter]

If I add a breakopint everything works fine. Here's a description why wro4j 
generates wrong urls and why it works
when there is a breakpoint at Wromanager#initAggregatedFolderPath().

The browser sends two requests (login.css and login.js) at the same time. The 
server then starts two threads (7 and 9).

  18:03:03.210 [http-bio-8080-exec-7] DEBUG ro.isdc.wro.manager.WroManager - request URL: http://localhost:8080/e4/wro/login.css
  18:03:03.211 [http-bio-8080-exec-9] DEBUG ro.isdc.wro.manager.WroManager - request URL: http://localhost:8080/e4/wro/login.js

In the 7-thread the Wromanager sets the aggregatedFolder to /wro/. But not in 
the 9-thread because the resource is js and not css.

  18:03:03.216 [http-bio-8080-exec-7] DEBUG ro.isdc.wro.manager.WroManager - WroManager: set aggregatedFolder: /wro/

Now there are two contexts one with aggregatedFolder=/wro/ (thread-7) and one 
with aggregatedFolder=null (thread 9)
A few millisenconds later the two threads inject the context into the 
CssUrlRewritingProcessor instance. The 7-thread sets the context first, after 
that the 9-thread does the same and overwrites the context from the 7-thread.

  18:03:03.373 [http-bio-8080-exec-7] DEBUG r.i.w.model.group.processor.Injector - processInjectAnnotation for: CssUrlRewritingProcessor
  18:03:03.373 [http-bio-8080-exec-9] DEBUG r.i.w.model.group.processor.Injector - processInjectAnnotation for: CssUrlRewritingProcessor
  18:03:03.373 [http-bio-8080-exec-7] DEBUG ro.isdc.wro.config.Context - get Context for correlationId: 75c1d58d-7f38-4cfa-bb96-ea69a4adad34
  18:03:03.373 [http-bio-8080-exec-9] DEBUG ro.isdc.wro.config.Context - get Context for correlationId: eb6c12e9-3818-44c2-93fd-811b5e3720a4
  18:03:03.374 [http-bio-8080-exec-7] DEBUG r.i.w.model.group.processor.Injector -  [OK] Injected ro.isdc.wro.model.resource.processor.impl.css.CssUrlRewritingProcessor -> Context
  18:03:03.374 [http-bio-8080-exec-9] DEBUG r.i.w.model.group.processor.Injector -  [OK] Injected ro.isdc.wro.model.resource.processor.impl.css.CssUrlRewritingProcessor -> Context

And that's the reason why aggregatedFolderPath in CssUrlRewritingProcessor is 
null and the code creates wrong urls.

  18:03:03.415 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - aggregatedFolderPath: null
  18:03:03.415 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - computedPrefix: 
  18:03:03.416 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - computed aggregatedPathPrefix 
  18:03:03.417 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - aggregatedPathPrefix: 
  18:03:03.418 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - cssUri: /resources/css/app-sprite.css, imageUrl ../images/sprites.png
  18:03:03.418 [http-bio-8080-exec-7] DEBUG r.i.w.m.r.p.i.c.CssUrlRewritingProcessor - computedImageLocation: /resources/images/sprites.png

See the full log: http://rasc.ch/problem.txt

For comparison here the full log with breakpoint on line 232 in WroManager 
(final String requestUri = request.getRequestURI();)
http://rasc.ch/breakpoint.txt
Because the debugger halts the css-thread the js-thread runs first and after 
releasing the breakpoint the css-thread runs.
This way they don't interfere each other.

Original comment by ralphschaer@gmail.com on 24 Jun 2012 at 4:23

[GoogleCodeExporter]

Thanks for detailed description. It clarifies the problem. I'll submit a fix 
for next release.

Original comment by alex.obj...@gmail.com on 24 Jun 2012 at 8:35

• Changed state: Accepted
• Added labels: Milestone-Release-1.4.7
• Removed labels: Milestone-Release-1.4.6

[GoogleCodeExporter]

Fixed in branch 1.4.x.

The injection in Context is not supported anymore. However, a new interface was 
created: ReadOnlyContext (Context implements ReadOnlyContext) which can be 
injected safely in any processor. The trick behind the fix, is that a Proxy is 
injected instead of actual Context object which is thread-safe.

Original comment by alex.obj...@gmail.com on 25 Jun 2012 at 7:26

• Changed state: Fixed

[GoogleCodeExporter]

I'm experiencing the same issue (randomly). Is there workaround for it until 
1.4.7 is released? I would consider this to be blocker and should be pushed as 
soon as possible.

Original comment by lystoc...@gmail.com on 29 Jun 2012 at 7:09

[GoogleCodeExporter]

1.4.7 will be released this week. There is a workaround, but require you to 
change a bit the implementation of CssUrlRewritingProcessor

Original comment by alex.obj...@gmail.com on 29 Jun 2012 at 7:12

[GoogleCodeExporter]

Thanks Alex, if work week is ended on Friday then it will be released today? :)
Do you know in which version is it regressed? E.g. is there more stable version 
in 1.4.x?

Original comment by lystoc...@gmail.com on 29 Jun 2012 at 7:44

[GoogleCodeExporter]

I'll try to release it today, but for sure it should be available until sunday. 
If this is critical for you, I would recommend to create a custom 
CssUrlRewritingProcessor which fixes the problem. I can give you more details 
if you need.

Original comment by alex.obj...@gmail.com on 29 Jun 2012 at 7:52

[GoogleCodeExporter]

Yes, please give the details, I will try those.

Original comment by lystoc...@gmail.com on 29 Jun 2012 at 8:43

[GoogleCodeExporter]

The fix is simple. Create a custom CssUrlRewritingProcessor with the same 
implementation as the original. Add the following changes:
- remove the context field (the one which has @Inject annotation).
- use Context.get() instead of the old field. More specifically, replace this 
line:

final String aggregatedPathPrefix = 
computeAggregationPathPrefix(context.getAggregatedFolderPath());

with

final String aggregatedPathPrefix = 
computeAggregationPathPrefix(Context.get().getAggregatedFolderPath());

The last step is to configure processorsFactory to use your custom 
CssUrlRewritingProcessor. This should be enough to make it work.

Original comment by alex.obj...@gmail.com on 29 Jun 2012 at 8:48

[GoogleCodeExporter]

Thanks Alex! Workaround worked.

Original comment by lystoc...@gmail.com on 29 Jun 2012 at 11:22

[GoogleCodeExporter]

I tested the latest code from github with my e4ds application. 
It works fine now. Thanks for fixing this so fast. 

Original comment by ralphschaer@gmail.com on 29 Jun 2012 at 2:07