Race condition on browser load when resolving getRgbStringViaBrowser() #1985
Description
Lightweight Charts™ Version: 5.0.9
This is a rather rare race condition on browser load when translating colours via getRgbStringViaBrowser(). I've only observed this during plugin development because there's nothing else really to load the page, and so LWC loads in very quickly. I don't think this is a particularly important issue to resolve - just a bit of an oddity when developing.
Steps/code to reproduce:
Replicating with LWC is tricky (adding a createPriceLine() to a series may occasionally work in a new plugin). Otherwise, the easiest way is to just use raw JS in a HTML page:
<!DOCTYPE html>
<html lang="en">
<head></head>
<body>
<script>
function getRgbStringViaBrowser(color) {
const element = document.createElement('div');
element.style.display = 'none';
// We append to the body as it is the most reliable way to get a color reading
// appending to the chart container or similar element can result in the following
// getComputedStyle returning empty strings on each check.
document.body.appendChild(element);
element.style.color = color;
const computed = window.getComputedStyle(element).color;
console.log(`${color} => ${computed}`);
document.body.removeChild(element);
return computed;
}
getRgbStringViaBrowser('red');
setTimeout(() => {
getRgbStringViaBrowser('red');
}, 100);
</script>
</body>
</html>Actual behavior:
All colours are converted to some browser default colour because it hasn't yet initialised all page state yet.
Expected behavior:
Colours to be properly converted.
Screenshots:
See the grey price backgrounds (even though price lines are different colours with no axisLabelColor specified, so the axis labels should fall back to the price line colour):
A simple 'fix' would be to just not cache the colour conversion if the browser is not ready yet. Wrong colours will be returned initially still, but repainting will probably happen soon after. I'm not sure what a better approach would be.
diff --git a/src/model/colors.ts b/src/model/colors.ts
index 315959994..7b18dbf2f 100644
--- a/src/model/colors.ts
+++ b/src/model/colors.ts
@@ -187,7 +187,9 @@ export class ColorParser {
(match[4] ? parseFloat(match[4]) : 1) as AlphaComponent,
];
- this._rgbaCache.set(color, rgba);
+ if (document.readyState === 'complete') {
+ this._rgbaCache.set(color, rgba);
+ }
return rgba;
}
I think complete is the only state that is safe, judging from the following:
<!DOCTYPE html>
<html lang="en">
<head></head>
<body>
<script>
function getRgbStringViaBrowser(color) {
const element = document.createElement('div');
element.style.display = 'none';
// We append to the body as it is the most reliable way to get a color reading
// appending to the chart container or similar element can result in the following
// getComputedStyle returning empty strings on each check.
document.body.appendChild(element);
element.style.color = color;
const computed = window.getComputedStyle(element).color;
console.log(`${document.readyState}: ${color} => ${computed}`);
document.body.removeChild(element);
return computed;
}
getRgbStringViaBrowser('red');
let i = 0;
const interval = setInterval(() => {
getRgbStringViaBrowser('red');
if (i++ > 100) {
clearInterval(interval);
}
}, 1);
</script>
</body>
</html>