Skip to content

Commit 328eaa0

Browse files
s0ph1es0ph1e
committed
Fix css handler bugs (#60)
* Fix similar css urls not updated #53, #54 * Fix similar css urls not updates, close #48 * Add test for ignore changing partially matched names in css files, #54 * Fix changing partially matched css urls, #54 * Remove unneeded nock usage in css handler tests * Fix typo * Change css regexp
1 parent e88abb2 commit 328eaa0

File tree

2 files changed

+94
-35
lines changed

2 files changed

+94
-35
lines changed

‎lib/file-handlers/css.js‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,18 @@
11
var _ = require('lodash');
2+
var format = require('util').format;
23
var getCssUrls = require('css-url-parser');
34
var utils = require('../utils');
45

6+
function changeExactlyMatchedUrl (text, oldUrl, newUrl) {
7+
// starts with ' " ( ends with ' " )
8+
var exactlyMatchedPattern = format('([\'"\(\s])%s([\'"\)\s])', _.escapeRegExp(oldUrl));
9+
var exactlyMatchedRegexp = new RegExp(exactlyMatchedPattern, 'g');
10+
text = text.replace(exactlyMatchedRegexp, function changeUrl (match, g1, g2) {
11+
return g1 + newUrl + g2;
12+
});
13+
return text;
14+
}
15+
516
function loadCss (context, resource) {
617
var url = resource.getUrl();
718
var filename = resource.getFilename();
@@ -17,7 +28,7 @@ function loadCss (context, resource) {
1728
resource.updateChild(childResource, loadedResource);
1829

1930
var relativePath = utils.getRelativePath(filename, loadedResource.getFilename());
20-
text = text.replace(cssUrl, relativePath);
31+
text = changeExactlyMatchedUrl(text, cssUrl, relativePath);
2132
}
2233
});
2334
});

‎test/unit/file-handlers/css-test.js‎

Lines changed: 82 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,38 +1,28 @@
11
require('should');
2-
var sinon = require('sinon');
3-
var nock = require('nock');
4-
var fs = require('fs-extra');
52
var Promise = require('bluebird');
3+
var sinon = require('sinon');
4+
require('sinon-as-promised')(Promise);
65
var Scraper = require('../../../lib/scraper');
76
var Resource = require('../../../lib/resource');
87
var loadCss = require('../../../lib/file-handlers/css');
98

10-
var testDirname = __dirname + '/.css-test';
119
var defaultScraperOpts = {
1210
urls: [ 'http://example.com' ],
13-
directory: testDirname
11+
directory: __dirname + '/.css-test'
1412
};
1513
var scraper;
1614

1715
describe('Css handler', function () {
1816

1917
beforeEach(function() {
20-
nock.cleanAll();
21-
nock.disableNetConnect();
2218
scraper = new Scraper(defaultScraperOpts);
2319
scraper.prepare();
2420
});
2521

26-
afterEach(function() {
27-
nock.cleanAll();
28-
nock.enableNetConnect();
29-
fs.removeSync(testDirname);
30-
});
31-
3222
describe('#loadCss(context, resource)', function() {
3323

3424
it('should not call loadResource if no sources in css', function(done) {
35-
var loadResourceSpy = sinon.spy(scraper, 'loadResource');
25+
var loadResourceSpy = sinon.stub(scraper, 'loadResource');
3626

3727
var po = new Resource('http://example.com', '1.css');
3828
po.setText('');
@@ -44,9 +34,7 @@ describe('Css handler', function () {
4434
});
4535

4636
it('should call loadResource once with correct params', function(done) {
47-
nock('http://example.com').get('/test.png').reply(200, 'OK');
48-
49-
var loadResourceSpy = sinon.spy(scraper, 'loadResource');
37+
var loadResourceSpy = sinon.stub(scraper, 'loadResource').resolves();
5038

5139
var po = new Resource('http://example.com', '1.css');
5240
po.setText('div {background: url(test.png)}');
@@ -59,11 +47,7 @@ describe('Css handler', function () {
5947
});
6048

6149
it('should call loadResource for each found source with correct params', function(done) {
62-
nock('http://example.com').get('/a.jpg').reply(200, 'OK');
63-
nock('http://example.com').get('/b.jpg').reply(200, 'OK');
64-
nock('http://example.com').get('/c.jpg').reply(200, 'OK');
65-
66-
var loadResourceSpy = sinon.spy(scraper, 'loadResource');
50+
var loadResourceSpy = sinon.stub(scraper, 'loadResource').resolves();
6751
var css = '\
6852
.a {background: url(a.jpg)} \
6953
.b {background: url(\'b.jpg\')}\
@@ -83,14 +67,10 @@ describe('Css handler', function () {
8367
});
8468

8569
it('should replace all sources in text with local files', function(done) {
86-
nock('http://first.com').get('/img/a.jpg').reply(200, 'OK');
87-
nock('http://first.com').get('/b.jpg').reply(200, 'OK');
88-
nock('http://second.com').get('/img/c.jpg').once().reply(200, 'OK');
89-
9070
var loadStub = sinon.stub(scraper, 'loadResource');
91-
loadStub.onFirstCall().returns(Promise.resolve(new Resource('http://first.com/img/a.jpg', 'local/a.jpg')));
92-
loadStub.onSecondCall().returns(Promise.resolve(new Resource('http://first.com/b.jpg', 'local/b.jpg')));
93-
loadStub.onThirdCall().returns(Promise.resolve(new Resource('http://second.com/img/c.jpg', 'local/c.jpg')));
71+
loadStub.onFirstCall().resolves(new Resource('http://first.com/img/a.jpg', 'local/a.jpg'));
72+
loadStub.onSecondCall().resolves(new Resource('http://first.com/b.jpg', 'local/b.jpg'));
73+
loadStub.onThirdCall().resolves(new Resource('http://second.com/img/c.jpg', 'local/c.jpg'));
9474

9575
var css = '\
9676
.a {background: url("http://first.com/img/a.jpg")} \
@@ -117,12 +97,10 @@ describe('Css handler', function () {
11797
});
11898

11999
it('should not replace the sources in text, for which loadResource returned null', function(done) {
120-
nock('http://second.com').get('/img/c.jpg').once().reply(200, 'OK');
121-
122100
var loadStub = sinon.stub(scraper, 'loadResource');
123-
loadStub.onFirstCall().returns(Promise.resolve(null));
124-
loadStub.onSecondCall().returns(Promise.resolve(null));
125-
loadStub.onThirdCall().returns(Promise.resolve(new Resource('http://second.com/img/c.jpg', 'local/c.jpg')));
101+
loadStub.onFirstCall().resolves(null);
102+
loadStub.onSecondCall().resolves(null);
103+
loadStub.onThirdCall().resolves(new Resource('http://second.com/img/c.jpg', 'local/c.jpg'));
126104

127105
var css = '\
128106
.a {background: url("http://first.com/img/a.jpg")} \
@@ -149,5 +127,75 @@ describe('Css handler', function () {
149127
done();
150128
}).catch(done);
151129
});
130+
131+
it('should replace all occurencies of the same sources in text with local files', function(done) {
132+
sinon.stub(scraper, 'loadResource').resolves(new Resource('http://example.com/img.jpg', 'local/img.jpg'));
133+
134+
var css = '\
135+
.a {background: url("http://example.com/img.jpg")} \
136+
.b {background: url("http://example.com/img.jpg")}\
137+
.c {background: url("http://example.com/img.jpg")}\
138+
';
139+
140+
var po = new Resource('http://example.com', '1.css');
141+
po.setText(css);
142+
143+
return loadCss(scraper, po).then(function(){
144+
var text = po.getText();
145+
var numberOfLocalResourceMatches = text.match(/local\/img.jpg/g).length;
146+
147+
text.should.not.containEql('http://example.com/img.jpg');
148+
text.should.containEql('local/img.jpg');
149+
numberOfLocalResourceMatches.should.be.eql(3);
150+
done();
151+
}).catch(done);
152+
});
153+
154+
it('should replace resource only if it completely equals to path (should not change partially matched names)', function(done) {
155+
// Next order of urls will be returned by css-url-parser
156+
// 'style.css', 'mystyle.css', 'another-style.css', 'image.png', 'another-image.png', 'new-another-image.png'
157+
158+
// Order of args for calling loadStub depends on order of css urls
159+
// first time it will be called with cssUrls[0], second time -> cssUrls[1]
160+
var loadStub = sinon.stub(scraper, 'loadResource');
161+
loadStub.onCall(0).resolves(new Resource('http://example.com/style.css', 'local/style.css'));
162+
loadStub.onCall(1).resolves(new Resource('http://example.com/mystyle.css', 'local/mystyle.css'));
163+
loadStub.onCall(2).resolves(new Resource('http://example.com/another_style.css', 'local/another_style.css'));
164+
loadStub.onCall(3).resolves(new Resource('http://example.com/image.png', 'local/image.png'));
165+
loadStub.onCall(4).resolves(new Resource('http://example.com/another-image.png', 'local/another-image.png'));
166+
loadStub.onCall(5).resolves(new Resource('http://example.com/new-another-image.png', 'local/new-another-image.png'));
167+
168+
var css = '\
169+
@import "style.css"; \
170+
@import \'style.css\'; \
171+
@import \'mystyle.css\'; \
172+
@import url(\'style.css\') ;\
173+
@import url(\'another-style.css\'); \
174+
.a {background: url("image.png")} ;\
175+
.b {background: url(image.png)}; \
176+
.c {background: url("another-image.png")};\
177+
.d {background: url(\'new-another-image.png\')};\
178+
';
179+
180+
var po = new Resource('http://example.com', '1.css');
181+
po.setText(css);
182+
183+
return loadCss(scraper, po).then(function(){
184+
var text = po.getText();
185+
186+
text.should.containEql('@import "local/style.css"');
187+
text.should.containEql('@import \'local/style.css\'');
188+
text.should.containEql('@import \'local/mystyle.css\'');
189+
text.should.containEql('@import url(\'local/style.css\')');
190+
text.should.containEql('@import url(\'local/another_style.css\')');
191+
192+
text.should.containEql('.a {background: url("local/image.png")}');
193+
text.should.containEql('.b {background: url(local/image.png)}');
194+
text.should.containEql('.c {background: url("local/another-image.png")}');
195+
text.should.containEql('.d {background: url(\'local/new-another-image.png\')}');
196+
197+
done();
198+
}).catch(done);
199+
});
152200
});
153201
});

0 commit comments

Comments
 (0)