From c96b90cc7567a8c5f7d8434732a96b5975d1a122 Mon Sep 17 00:00:00 2001 From: Matthew Beale Date: Thu, 14 Dec 2023 21:34:31 -0500 Subject: [PATCH] Compare offsetHeight/getComputedStyle at scale At very large heights based on many child elements, Chrome and even Firefox begin to diverge the computed height values from what you see at lower scales. This makes their comparison with offsetHeight less reliable at high scales. However, at large height values the exactness of the height measurement also matters less. At the end, only a limited level of precision is desired in the scale value and later code throws away noise at the lower end. So, use a more forgiving comparison methodology (% shift in the change) vs requiring the two values to be only a single digit off. --- addon/-private/utils/element.js | 27 +++++-- tests/unit/-private/element-utils-test.js | 92 +++++++++++++++++++++++ 2 files changed, 112 insertions(+), 7 deletions(-) diff --git a/addon/-private/utils/element.js b/addon/-private/utils/element.js index 272abda29..b3c06dd90 100644 --- a/addon/-private/utils/element.js +++ b/addon/-private/utils/element.js @@ -30,6 +30,10 @@ export function closest(el, selector) { return null; } +function parseComputedStyleHeightToPixels(computedHeightStyleValue) { + return Number(computedHeightStyleValue.substring(0, computedHeightStyleValue.length - 2)); +} + /* * Calculate the scale of difference between an element's logical height * (the offsetHeight or getComputedStyle height) compared to its rendered @@ -61,18 +65,27 @@ export function getScale(element) { } let computedHeightStyleValue = window.getComputedStyle(element).height; - let computedHeightInPixels = Number( - computedHeightStyleValue.substring(0, computedHeightStyleValue.length - 2) - ); + let computedHeightInPixels = parseComputedStyleHeightToPixels(computedHeightStyleValue); let offsetHeight = element.offsetHeight; if (isNaN(computedHeightInPixels)) { computedHeightInPixels = offsetHeight; - } else if (Math.abs(computedHeightInPixels - offsetHeight) >= 1) { - throw new Error( - "EmberTable's getScale() utility can only work on elements where height as derived from getComputedStyle is reliable. Usually this means padding is present on the target element." - ); + } else { + let [min, max] = [computedHeightInPixels, offsetHeight].sort(); + let difference = max - min; + + /* + * This assertion once used a static comparison of the two + * values to make sure they were within 1 number of each + * other. Chrome has a bug when height is a number 500k + * that seems to require a wider range. + */ + if ((max <= 500000 && difference >= 1) || (max > 500000 && (max - min) / max > 0.00001)) { + throw new Error( + "EmberTable's getScale() utility can only work on elements where height as derived from getComputedStyle is reliable. This error flags that offsetHeight and getComputedStyle disagree on the target element dimensions. This can be caused by padding, thead elements, and other cases." + ); + } } if (computedHeightInPixels === renderedHeight) { diff --git a/tests/unit/-private/element-utils-test.js b/tests/unit/-private/element-utils-test.js index e48a33490..0c372abb8 100644 --- a/tests/unit/-private/element-utils-test.js +++ b/tests/unit/-private/element-utils-test.js @@ -57,4 +57,96 @@ module('Unit | Private | element', function(hooks) { getScale(div); }); }); + + test('can get the scale of element with table header', function(assert) { + let table = document.createElement('table'); + table.style.borderSpacing = '0'; + + let thead = document.createElement('thead'); + table.append(thead); + + let row = document.createElement('tr'); + row.style.lineHeight = '8.5px'; + row.style.height = '8.5px'; + let cell = document.createElement('td'); + cell.style.padding = '0px'; + cell.textContent = 'header cell'; + row.append(cell); + thead.append(row); + + let tbody = document.createElement('tbody'); + tbody.style.borderTopWidth = '1px'; + tbody.style.borderTopStyle = 'solid'; + table.append(tbody); + + for (let i = 0; i < 50; i++) { + let row = document.createElement('tr'); + row.style.lineHeight = '40px'; + row.style.height = '40px'; + let cell = document.createElement('td'); + cell.style.padding = '0px'; + cell.textContent = `cell ${i}`; + row.append(cell); + tbody.append(row); + } + + this.element.append(table); + + assert.equal(getScale(table), 1, 'scale on a simple element is correct'); + + table.style.transform = 'scale(0.5)'; + + assert.equal(getScale(table), 2, 'scale on a scaled element is correct'); + }); + + /* + * For very large numbers of items and rows, Chrome seems to start to + * yield internally inconsistent values between computed height and + * offset height. Assert that we've covered that case. + */ + test('can get the scale of element with table header and extraordinary item count and height', function(assert) { + let table = document.createElement('table'); + table.style.borderSpacing = '0'; + + let thead = document.createElement('thead'); + table.append(thead); + + let row = document.createElement('tr'); + row.style.lineHeight = '1.5px'; + row.style.height = '1.5px'; + let cell = document.createElement('td'); + cell.style.padding = '0px'; + cell.textContent = 'header cell'; + row.append(cell); + thead.append(row); + + let tbody = document.createElement('tbody'); + tbody.style.borderTopWidth = '1px'; + tbody.style.borderTopStyle = 'solid'; + table.append(tbody); + + /* + * This pushes the height of the box above 7 digits, which + * seems to reliably trigger the internal bug is triggered. + * It has been observed on macOS at a lower value of 500k. + */ + for (let i = 0; i < 250; i++) { + let row = document.createElement('tr'); + row.style.lineHeight = '4000px'; + row.style.height = '4000px'; + let cell = document.createElement('td'); + cell.style.padding = '0px'; + cell.textContent = `cell ${i}`; + row.append(cell); + tbody.append(row); + } + + this.element.append(table); + + assert.equal(getScale(table), 1, 'scale on a simple element is correct'); + + table.style.transform = 'scale(0.5)'; + + assert.equal(getScale(table), 2, 'scale on a scaled element is correct'); + }); });