Schepp / CSS-Filters-Polyfill

This polyfill takes the official CSS filters syntax and translates it to the different equivalent techniques that the browsers know for those effects
MIT License
762 stars 66 forks source link

Remove cache feature to remove indirection #12

Closed mariuswilms closed 11 years ago

mariuswilms commented 11 years ago

It doesn't work reliably and the double refresh adds another indirection layer, which can cause confusion. Also the parsing isn't very time consuming in my measurements.

This was the error I was getting using the caching feature in Firefox 24 on OSX:

[16:43:20.040] NS_ERROR_FILE_CORRUPTED: Component returned failure code: 0x8052000b 
(NS_ERROR_FILE_CORRUPTED) [nsIDOMStorage.getItem] 
@ http://example.org/v5b33fae/core/js/compat/css-filters.js:13
Schepp commented 11 years ago

It is not the parsing that is time consuming, but the AJAX-request to the server that needs to be done to "refetch" the CSS file. Parsing is always executed, even when using a cached resource.

Don't know what happened with you Firefox though. I am doing nothing exotic.

mariuswilms commented 11 years ago

"Best practice" is to set a high "Expires" on your assets. That's what I'm doing and this also explains why I don't see any additional overhead when removing the cache feature. Basically what you are doing with caching to localStorage is duplicating already existent browser features (caching). I'd vote for removing the duplication and instead of explaining the odds of localStorage caching in the README, request people to follow best practice (maybe they already do) and set a high Expire on their assets.

Update:

To explain the request/response flow a bit further: Initial page load -> browser loads stylesheets -> browser sees Expires header and caches stylesheets ->polyfilter runs and retrieves stylesheets -> browser servers stylesheets from browser cache.

Schepp commented 11 years ago

Although there is truth in what you say there are some issues on mobile devices that still hamper native browser cache experience. Main problem being cache access speed: http://www.mobify.com/blog/smartphone-localstorage-outperforms-browser-cache/.

So how about making the feature configurable?

mariuswilms commented 11 years ago

I don't have any benchmarks on my hand. However I would doubt the results from the article you've linked. Although it is an interesting read. The author even admits to have excluded the read-from-disk timings for localStorage. Another commenter notes that these probably would impact the benchmarks. Altogether I don't see why accessing localStorage should be significantly faster than the browser cache.

Schepp commented 11 years ago

Sorry for the delay. Was interrrupted by the Fronteers conference in the Netherlands and afterwards needed to catch up on things. Merging in 3, 2, 1...