Fixes firefox copy paste issue

[gstjepan2]

Fixes firefox copy paste issue where copy pasting from src adds extra newline.
Closes: #141464

[rustbot]

r? @GuillaumeGomez

rustbot has assigned @GuillaumeGomez.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[rustbot]

Some changes occurred in HTML/CSS/JS.

cc @GuillaumeGomez, @jsha, @lolbinarycat

[GuillaumeGomez]

With this fix, this code:

fn foo() {}

fn bar() {}

would become:

fn foo() {}
fn bar() {}

Which definitely sounds like a new bug.

[gstjepan2]

You're right it it also like looks I've went with the wrong approach.

Just settings the data to text/plain seems to fix the issue

if (navigator.userAgent.includes('Firefox')) {
  document.addEventListener('copy', function(e) {
    const text = nonnull(window.getSelection()).toString();    
    nonnull(e.clipboardData).setData('text/plain', text);
    e.preventDefault();
  });
} 

I'll amend the commit with this

[lolbinarycat]

1. looking for the string "Firefox" is correct because safari also puts "Mozilla" in its user agent string
2. getSelection can only return null if in a contextless environment like a detached iframe (in which case we shouldn't be getting copy events) and clipboardData can only be null for manually constructed events, so the usage of nonnull here looks correct.

It would be nice to fix this without user agent detection, but I don't see an obvious cause, the pre element only contains one set of linebreaks, and there are no extraneous whitespace nodes. The issue is the same with js enabled and disabled.

If this fixes the issue, I guess that's good.

At the very least we should probably have a comment that says why we are doing this, like "workaround for "

[gstjepan2]

Added a workaround for comment

[GuillaumeGomez]

I'm really not convinced it's the right fix. Even more considering it's only for one specific use-case. Wouldn't adding the right tags to the code viewer element be a better alternative?

[gstjepan2]

I agree, but wouldn't that be a too big of a (breaking?) change just for copy pasting issue in firefox?

[GuillaumeGomez]

I don't think so? But maybe we have something different in mind. Adding an attribute on pre.rust or on pre.rust > code to tell the web browser how to copy-paste so we don't need a JS fix should be possible, no?

[gstjepan2]

Oh gotcha, i am not aware of any clean way of doing that though, expecially without js not sure if copy paste behaviour can be modified

[lolbinarycat]

Hold on, have we verified this is only an issue on firefox? It doesn't seem to happen in chrome but someone should probably also check safari at least.