1801186 - On copy, non-breaking spaces used for padding should be converted to regular spaces

Status: RESOLVED FIXED

(Core :: DOM: Serializers, defect)

Description

[Pierre de La Morinerie]

Steps to reproduce:

Given the following code snippet, indented using non-breaking spaces:

SELECT column_names
    FROM table_name
    WHERE column_name IS NOT NULL;

1. Select the three lines of this snippet,
2. Copy the selected text to the clipboard,
3. Paste the text in any external SQL shell (like psql), and execute the command.

Actual results:

The SQL shell raises an error, because unexpected non-breaking spaces are present in the pasted text.

Expected results:

The SQL command should be properly executed by the shell.

Notes:

The problem was reported for code blocks generated using Confluence Wiki (see https://www.reddit.com/r/firefox/comments/yxmrt9/firefox_is_now_changing_white_space_indents_into/).

Using non-breaking spaces for code indentation is clearly a mis-use (white-space: pre would be the correct solution); but some websites or wiki software in the wild seem to rely on the browser to convert NBSPs to regular spaces.

Since this behavior changed in Firefox 107 (bug 1769534), this is technically a regression.

Comment 1

[Pierre de La Morinerie]

A solution would be to consider non-breaking spaces adjacent to a string beginning or line-break to be non-sense (i.e. without typographic use), and convert those to regular spaces.

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The Bugbug bot thinks this bug should belong to the 'Core::Layout: Text and Fonts' component, and is moving the bug to that component. Please correct in case you think the bot is wrong.

Comment 3

[Yuri Khan]

I had warned about this particular case way back in bug 359303 comment 144.

Comment 4

[BugBot [:suhaib / :marco/ :calixte]]

The bug has a release status flag that shows some version of Firefox is affected, thus it will be considered confirmed.

Comment 5

[Ksenia Berezina [:ksenia]]

There is a webcompat report about a similar issue on https://www.javatpoint.com/how-to-make-a-javascript-game, where copying a code sample into an IDE results in unexpected non-breaking spaces being present since bug1769534.

Comment 6

[Henri Sivonen (:hsivonen)]

Pierre, are you planning to fix this?

Comment 7

[Pierre de La Morinerie]

I'd like too, but there's two limiting factors for me:

1. In a few days I'll have a newborn at home, which will limit my capability for coding for at least a few weeks,
2. I tried to quickly draft the tweaks required for the algorithm to work correctly, and at this point it seems regexp would be better suited to the task. Except I'm not sure if this part of the serializer supports regexps, neither of the performance implications.

I could use some help on 2., and I'm unsure if the timing will be ok regarding 1. :)

Comment 8

[Brian Mathis]

This is causing huge issues for me, and I suspect many others as well, as nbsp is very commonly used in ticketing systems (e.g. Jira) for code indentation. This has rendered my entire history of Jira tickets with SQL code as non-functional. Given that this was pushed during the holiday season, I suspect there will be an influx of issues reported as people both get the new release and return to work.

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Henri, I'm wondering if we can get some help on this? Should we consider reverting the regressing change due to the unexpected webcompat fallout until we have more time to find a better fix for this bug?

Comment 10

[Henri Sivonen (:hsivonen)]

I will try to see what I can do to find out at least the magnitude of the change needed to either write a patch or to decide to back out.

Comment 11

[Henri Sivonen (:hsivonen)]

I think we should back this out. Sorry about not investigating this beyond what the comments claimed sooner. I thought the issue was about indent, which would have been recoverable, but if a site does what https://www.javatpoint.com/how-to-make-a-javascript-game does an replaces all spaces with NBSPs, special-casing line-starting sequences isn't enough.

RyanVM, can you back this out on all relevant branches?

Comment 12

[Henri Sivonen (:hsivonen)]

FWIW, I was unable to confirm the problem with Confluence using the instance of Atlassian-hosted Confluence that I have access to. It's possible that a past version of the Confluence editor has converted indent spaces to NBSPs and those remain on existing wikis even if the latest version of the editor doesn't do so.

Comment 13

[Henri Sivonen (:hsivonen)]

If we still want to try to re-introduce some NBSP preservation later, we should 1) replace all NBSPs with spaces between the start of the line and the first character on the line that's neither a regular space nor a NBSP and 2) let the behavior bake for longer as Nightly-only to find out how common it is for sites to replace all spaces in code with NBSPs (which leads to the pattern that in other contexts is exactly what we wanted to keep as NBSP in the first place: non-space, NBSP, non-space).

Comment 14

[Ryan VanderMeulen [:RyanVM]]

We can backout from m-c for Fx110, but we've already build the Fx109 RC. Is this something you think we should respin the RC for? Or a dot release ride-along?

Comment 15

[Henri Sivonen (:hsivonen)]

I'm shy to request an RC respin for this, but please take the backout as a ride-along if you respin RC for another reason or for a dot release.

Comment 16

[Ryan VanderMeulen [:RyanVM]]

Backed out for 109.0rc2. Will backout from m-c as well for 110+.
https://hg.mozilla.org/releases/mozilla-release/rev/997f4574962f

Comment 17

[Ryan VanderMeulen [:RyanVM]]

Backed out for 110+ as well. We can take the remaining follow-up investigation back to the original bug.
https://hg.mozilla.org/integration/autoland/rev/ad3d7904355589e6b14c1d0136d30bf85d646cce