EuiCodeBlock doesn't have a "Copy" button is Safari when rendered in EuiBasicTable's expandable row

[nikitaindik]

Describe the bug
If you pass an isCopyable prop to EuiCodeBlock it should render a text copy button. However, in Safari, the copy button isn't shown when you use EuiCodeBlock as an expandable table row. But it does work properly in other cases.

Environment and versions

• EUI version: 77.2.2
• Kibana version: 8.9.0
• Browser: Safari 16.4 (18615.1.26.110.1)
• Operating System: MacOS 13.3.1 (a)

To Reproduce
Steps to reproduce the behavior:

• Checkout Kibana's branch: https://github.com/nikitaindik/kibana/tree/scrollable-callout (not yet merged, scheduled for v8.9)
• Run Kibana locally
• Go to Security Solution Rule Management page in Safari (http://localhost:5601/kbn/app/security/rules/management)
• Click a button to load prebuilt rules
• Once the rules are loaded, find Endpoint Security rule and click on it
• At the top of Endpoint Security rule page you'll see a yellow warning callout, note that it has a copy button
• Scroll to the bottom of the page and select Execution Results tab
• There should be an expandable warning, expand it. You'll see the warning message text without the copy button.

Expected behavior
TextBlock's copy button should be visible in Safari when TextBlock is rendered within EuiBasicTable's expandable row.

Screenshots
❌ Safari: Expandable row in Execution log - no copy button

✅ Chrome: Expandable row in Execution log - copy button present

✅ Safari: Warning callout - copy button present

✅ Chrome: Warning callout - copy button present

---

Additional context
I tried to dig into this bug a little, and here's what I found.

EuiCodeBlock component uses useInnerText hook under the hood. As I understand, useInnerText works like this: you give it a ref, and it reads that ref's .innerText value initially and on every update.

eui/src/components/inner_text/inner_text.tsx

Line 19 in c85913b

export function useInnerText(

And there's conditional logic: if there's no text to copy (innerText value is falsy) - do not render the button. That's what happens in Safari.

eui/src/components/code/code_block_copy.tsx

Line 34 in 8081fda

const showCopyButton = isCopyable && textToCopy;

For some reason in Safari, during an initial render, the value of innerText is an empty string (Safari bug?). But if you re-query it within a few moments, you'll get the proper value. useInnerText doesn't get notified when innerText becomes readable, so it thinks the value stays ''. Interestingly, textContent and innerHTML values can be read right away.

I tried logging values of innerText, textContent and innerHTML immediately after expanding the row.

❌ Safari: innerText is empty initially, but has value if you read it a little later. textContent and innerHTML are readable right away.

✅ Chrome: all three properties can be read right away

Safari: If you trigger a text update, useInnerText's observer would fire and you'll see the copy button.

Screen.Recording.2023-05-11.at.15.47.24.mov

---

I also tried modifying EuiBasicTable code to change how expanded row is rendered. If you remove either EuiTableRow wrapper or EuiTableRowCell wrapper, the copy button is rendered!

eui/src/components/basic_table/basic_table.tsx

Line 1036 in dc3fcbc

<EuiTableRow

✅ Safari: Without EuiTableRow - no opening animation, but copy button is rendered

So maybe it's connected to Safari handling animated elements differently somehow 🤷‍♀️

---

Possible fix ideas:

• Use textContent instead of innerText, so that it's compatible with Safari behavior?
• Remove animation for Safari?
• Allow passing text to copy as a prop to EuiCodeBlock?

[cee-chen]

Thank you so much for the detailed bug report, Nikita! It's seriously appreciated! I also love the time you took to grok the underlying issue & to suggest fixes.

It looks like useInnerText (sort of) already tries to fall back to textContent:

eui/src/components/inner_text/inner_text.tsx

Lines 29 to 36 in c85913b

	// Check for `innerText` implementation rather than a simple OR check
	// because in real cases the result of `innerText` could correctly be `null`
	// while the result of `textContent` could correctly be non-`null` due to
	// differing reliance on browser layout calculations.
	// We prefer the result of `innerText`, if available.
	'innerText' in node
	? node.innerText
	: node.textContent || innerTextFallback

I wonder if the comment in the above linked code still applies - it's possible it doesn't and we can now simply do a innerText || textContent. However, we'd probably need to perform a wide swathe of QA tests to ensure this doesn't regress behavior for other browsers in other components.

The other optional vectors for failure are:

• Mutation observer is not behaving as expected
• Stale refs/rerender cycles

I think your third suggestion, providing a copy fallback for EuiCodeBlock, would be a last resort option & will work, but we should try and figure out why Safari is failing first.

[cee-chen]

@tkajtoch would this be something you'd be interested in investigating / spiking out since you already potentially have another fix in the work for a separate bug w/ EuiCodeBlock's copy behavior?

[1Copenut]

I reproduced this in a minimal CodeSandbox if it's helpful for hacking against. https://codesandbox.io/s/stupefied-galileo-o8kn75?file=/demo.tsx

[1Copenut]

@tkajtoch It's a long shot, but I found a SO question Reactjs web app - MutationObserver use in Safari browser not as expected that has some of the same extenuating circumstances as this issue. The OP could see the expected outcome when they inspected their text area, couple of others.

Might be nothing, might be a lead. 🤞

[tkajtoch]

There are a few differences between innerText and textContent we should be aware of, but as long as we're passing a ref to an element that doesn't contain any hidden elements or <br> tags we should be fine to just use it instead.

Actually, because of that behavior I'd like to check if this issue isn't caused by asynchronous style updates that Safari somehow takes longer to process and extract text from child nodes.