Skip to content

Commit 88e7a74

Browse files
tobiasso85matz3
andauthored
[BREAKING] Only rewrite image paths for RTL-variant when files exist (#162)
Only rewrite img path to img-RTL in RTL (right-to-left) CSS if image file is present in img-RTL folder. Don't rewrite img paths with a protocol (http/https/data/...) or starting with a slash (`/`). BREAKING CHANGE: This affects the output of the `rtl` (right-to-left) variant that is created by applying several mechanisms to create a mirrored variant of the CSS to be used in locales that use a right to left text direction. One aspect is adopting urls which contain an `img` folder in the path. Before this change, all urls have been changed to use a `img-RTL` folder instead. This allows mirrored images to be used, but it also requires an images to be available on that path even when the original image should be used (e.g. for a logo). With this change: - Urls are only adopted in case an `img-RTL` variant of that file exists - Urls with a protocol (http/https/data/...) or starting with a slash (`/`) are not adopted anymore Co-authored-by: Matthias Osswald <[email protected]>
1 parent 41b9ba6 commit 88e7a74

16 files changed

+519
-49
lines changed

lib/index.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const fileUtilsFactory = require("./fileUtils");
1313
// Plugins
1414
const ImportCollectorPlugin = require("./plugin/import-collector");
1515
const VariableCollectorPlugin = require("./plugin/variable-collector");
16+
const UrlCollector = require("./plugin/url-collector");
1617

1718
// Workaround for a performance issue in the "css" parser module when used in combination
1819
// with the "colors" module that enhances the String prototype.
@@ -212,7 +213,7 @@ Builder.prototype.build = function(options) {
212213
resolve(tree);
213214
}
214215
});
215-
}).then(function(tree) {
216+
}).then(async function(tree) {
216217
const result = {};
217218

218219
result.tree = tree;
@@ -222,10 +223,11 @@ Builder.prototype.build = function(options) {
222223
importMappings: mFileMappings[filename]
223224
});
224225
const oVariableCollector = new VariableCollectorPlugin(options.compiler);
226+
const oUrlCollector = new UrlCollector();
225227

226228
// render to css
227229
result.css = tree.toCSS(Object.assign({}, options.compiler, {
228-
plugins: [oImportCollector, oVariableCollector]
230+
plugins: [oImportCollector, oVariableCollector, oUrlCollector]
229231
}));
230232

231233
// retrieve imported files
@@ -242,6 +244,22 @@ Builder.prototype.build = function(options) {
242244
if (options.rtl) {
243245
const RTLPlugin = require("./plugin/rtl");
244246
oRTL = new RTLPlugin();
247+
248+
const urls = oUrlCollector.getUrls();
249+
250+
const existingImgRtlUrls = (await Promise.all(
251+
urls.map(async ({currentDirectory, relativeUrl}) => {
252+
const relativeImgRtlUrl = RTLPlugin.getRtlImgUrl(relativeUrl);
253+
if (relativeImgRtlUrl) {
254+
const resolvedImgRtlUrl = path.posix.join(currentDirectory, relativeImgRtlUrl);
255+
if (await that.fileUtils.findFile(resolvedImgRtlUrl, options.rootPaths)) {
256+
return resolvedImgRtlUrl;
257+
}
258+
}
259+
})
260+
)).filter(Boolean);
261+
262+
oRTL.setExistingImgRtlPaths(existingImgRtlUrls);
245263
}
246264

247265
if (oRTL) {

lib/plugin/rtl.js

Lines changed: 44 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
"use strict";
22

33
const less = require("../thirdparty/less");
4+
const path = require("path");
5+
const url = require("url");
46

57
const cssSizePattern = /(-?[.0-9]+)([a-z]*)/;
68
const percentagePattern = /^\s*(-?[.0-9]+)%\s*$/;
@@ -202,11 +204,13 @@ const converterFunctions = {
202204
ruleNode.value.value.forEach(swapLeftRight);
203205
},
204206
url: function(ruleNode) {
205-
ruleNode.value.value.forEach(function(valueObject) {
207+
ruleNode.value.value.forEach((valueObject) => {
206208
if (valueObject.type === "Url") {
207-
replaceUrl(valueObject);
209+
this.replaceUrl(valueObject);
208210
} else if (valueObject.type === "Expression") {
209-
valueObject.value.forEach(replaceUrl);
211+
valueObject.value.forEach((childValueObject) => {
212+
this.replaceUrl(childValueObject);
213+
});
210214
}
211215
});
212216
},
@@ -566,14 +570,6 @@ function processCursorNode(node) {
566570
}
567571
}
568572

569-
function replaceUrl(node) {
570-
if (node.type === "Url") {
571-
modifyOnce(node, "replaceUrl", function(urlNode) {
572-
urlNode.value.value = urlNode.value.value.replace(urlPattern, urlReplacement);
573-
});
574-
}
575-
}
576-
577573
function endsWith(str, suffix) {
578574
return str.indexOf(suffix, str.length - suffix.length) !== -1;
579575
}
@@ -588,10 +584,15 @@ function modifyOnce(node, type, fn) {
588584
}
589585
}
590586

587+
/**
588+
*
589+
* @constructor
590+
*/
591591
const LessRtlPlugin = module.exports = function() {
592592
/* eslint-disable new-cap */
593593
this.oVisitor = new less.tree.visitor(this);
594594
/* eslint-enable new-cap */
595+
this.resolvedImgRtlPaths = [];
595596
};
596597

597598
LessRtlPlugin.prototype = {
@@ -611,5 +612,37 @@ LessRtlPlugin.prototype = {
611612
}
612613

613614
return ruleNode;
615+
},
616+
replaceUrl: function(node) {
617+
if (node.type !== "Url") {
618+
return;
619+
}
620+
modifyOnce(node, "replaceUrl", (urlNode) => {
621+
const imgPath = urlNode.value.value;
622+
const parsedUrl = url.parse(imgPath);
623+
if (parsedUrl.protocol || imgPath.startsWith("/")) {
624+
// Ignore absolute urls
625+
return;
626+
}
627+
const imgPathRTL = LessRtlPlugin.getRtlImgUrl(imgPath);
628+
if (!imgPathRTL) {
629+
return;
630+
}
631+
const resolvedUrl = path.posix.join(urlNode.currentFileInfo.currentDirectory, imgPathRTL);
632+
if (this.existingImgRtlPaths.includes(resolvedUrl)) {
633+
urlNode.value.value = imgPathRTL;
634+
}
635+
});
636+
},
637+
setExistingImgRtlPaths: function(existingImgRtlPaths) {
638+
this.existingImgRtlPaths = existingImgRtlPaths;
639+
}
640+
};
641+
642+
LessRtlPlugin.getRtlImgUrl = function(url) {
643+
if (urlPattern.test(url)) {
644+
return url.replace(urlPattern, urlReplacement);
645+
} else {
646+
return null;
614647
}
615648
};

lib/plugin/url-collector.js

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
"use strict";
2+
3+
const path = require("path");
4+
const url = require("url");
5+
const less = require("../thirdparty/less");
6+
7+
const urlNodeNames = {
8+
"background": true,
9+
"background-image": true,
10+
"content": true,
11+
"cursor": true,
12+
"icon": true,
13+
"list-style-image": true
14+
};
15+
16+
/**
17+
* @constructor
18+
*/
19+
const UrlCollectorPlugin = module.exports = function() {
20+
/* eslint-disable-next-line new-cap */
21+
this.oVisitor = new less.tree.visitor(this);
22+
this.urls = new Map();
23+
};
24+
25+
UrlCollectorPlugin.prototype = {
26+
isReplacing: false,
27+
isPreEvalVisitor: false,
28+
run: function(root) {
29+
return this.oVisitor.visit(root);
30+
},
31+
visitRule: function(ruleNode) {
32+
if (urlNodeNames[ruleNode.name]) {
33+
this.visitUrl(ruleNode);
34+
}
35+
},
36+
visitUrl: function(ruleNode) {
37+
for (const valueObject of ruleNode.value.value) {
38+
if (valueObject.type === "Url") {
39+
this.addUrlFromNode(valueObject);
40+
} else if (valueObject.type === "Expression") {
41+
for (const node of valueObject.value) {
42+
this.addUrlFromNode(node);
43+
}
44+
}
45+
}
46+
},
47+
addUrlFromNode: function(node) {
48+
if (node.type === "Url") {
49+
const relativeUrl = node.value.value;
50+
51+
const parsedUrl = url.parse(relativeUrl);
52+
// Ignore urls with protocol (also includes data urls)
53+
// Ignore server absolute urls
54+
if (parsedUrl.protocol || relativeUrl.startsWith("/")) {
55+
return;
56+
}
57+
58+
const {currentDirectory} = node.currentFileInfo;
59+
60+
const resolvedUrl = path.posix.join(currentDirectory, relativeUrl);
61+
this.urls.set(resolvedUrl, {currentDirectory, relativeUrl});
62+
}
63+
},
64+
getUrls: function() {
65+
return Array.from(this.urls.values());
66+
}
67+
};

package-lock.json

Lines changed: 118 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
"eslint-config-google": "^0.14.0",
9393
"graceful-fs": "^4.2.6",
9494
"mocha": "^8.3.1",
95-
"nyc": "^15.1.0"
95+
"nyc": "^15.1.0",
96+
"sinon": "^9.2.4"
9697
}
9798
}

0 commit comments

Comments
 (0)