Bug 27523 – Don't convert soft hyphens in selections into spaces

• Component: Layout and Rendering
• Status: RESOLVED INVALID

[Adam Langley]

Soft hyphens are code points which suggest optimal line-breaking points in words. Because they aren't usually part of a TextBox, we are currently converting all of them into spaces when getting the text of a selection.

With this patch, soft-hypens which aren't rendered are omitted in the selection.

[Adam Langley]

Created attachment 33222 [details]
patch

[Eric Seidel (no email)]

Comment on attachment 33222 [details]
patch

Need expected results for the layout test.

+        var e = window.eventSender;
+        e.mouseMoveTo(3, 5);
+        e.mouseDown();
+        for (var x = 4; x < 300; x += 20) {
+          e.mouseMoveTo(x, 5);
+        }
+        e.mouseUp();

can probably be replaced by something like this:

var sel = window.getSelection()
var p = document.getElementsByTagName("p")[0]; // an id lookup would be cleaner.
sel.selectAllChildren(p);

WebKit only way for selecting part of a node:
sel.setBaseAndExtent(p, 0, p, 10);

You can also create ranges and add them to the selection with sel.addRange().  In FF the only way to manipulate the selection is through Ranges:

var range = document.createRange();
range.setStart(p, 0);
range.setEnd(p, 10);
sel.removeAllRanges();
sel.addRange(range);

http://www.w3.org/TR/DOM-Level-2-Traversal-Range/ranges.html
Yes, the Range API is insane.

No need to do all the style stuff if you use Range/Selection APIs to create the selection.

I'm not really sure about the code change.  I would probably have put it into some sort of inline:
atRunEndOrSoftHyphen(str, nextRunStart, runEnd)

I'm not actually sure what the change is doing though, so difficult for me to name said inline. ;)

r- for lack of expected results.

[Adam Langley]

Created attachment 33234 [details]
patch

[Peter Arrenbrecht]

I just realized that Firefox copies the soft-hyphens as 0xad, which in gedit serves as a soft word-break location. Furthermore, it does _not_ copy shown hyphens, as you suggest doing in the webkit patch. And I don't think one should, because the paste target should handle wrapping, I think.

[Alexey Proskuryakov]

Comment on attachment 33234 [details]
patch

This looks like a duplicate of bug 11154 and/or 26774 to me. Please search for existing Bugzilla bugs when submitting new ones.

We usually check whether the test is running under DRT to avoid console spew if it's not. Also, it's slightly better to call dumpAsText as early as possible, to ease investigation in case something in the test suddenly starts raising exceptions.

if (window.layoutTestController)
    layoutTestController.dumpAsText();

The ChangeLog doesn't explain changes to charset-xuser-defined-expected.txt. Why are these safe?

r- for the reasons given by Peter Arrenbrecht above.

[Adam Langley]

Peter:

> Furthermore, it does _not_ copy shown hyphens, as you suggest doing in the webkit patch.

Substituting spaces for ­ is clearly wrong, which is why I got rid of it. If people think that we should keep the ­'s, then I can make that change. However, there's no way that we should keep the ­'s when not rendered and omit them when rendered as you suggest. Also, I can't replicate this behaviour in Firefox - it copies ­'s, rendered or not, for me.

[Peter Arrenbrecht]

Adam,

> Substituting spaces for ­ is clearly wrong, which is why I got rid of it.

Agreed.

> However, there's no way that we should keep the ­'s when not rendered and
> omit them when rendered as you suggest.

That is not what I meant to suggest. I suggest keeping _all_ ­'s as 0xad, as Firefox does, irrespective of whether they are rendered or not.

> Also, I can't replicate this behaviour
> in Firefox - it copies ­'s, rendered or not, for me.

I'm on Ubuntu Jaunty. When I copy in Firefox and then do

$ xclip -o -selection clipboard | hexdump -C

I get the all the ­ chars as 0xad. If I paste into gedit and turn line wrapping on there, when I resize the width of gedit, it clearly breaks the long words at the 0xad locations (although without adding a hyphen).

Referring again to http://recoveringphysicist.com/11/soft-hyphens, if I copy the last two words of the demo paragraph in Firefox and hexdump, I get:

00000000  75 6e ad 70 72 65 ad 6d  65 64 69 ad 74 61 74 65  |un.pre.medi.tate|
00000010  64 20 75 6e 69 ad 6c 61  74 65 72 ad 61 6c        |d uni.later.al|

Although if I paste this into a terminal, the 0xad's are rendered as spaces. Also if I paste into vim. So it's not entirely clear to me that what Firefox does really is desirable.

As of r71787, soft hyphens aren’t converted into spaces when converting to plain text. They are left as-is. This makes sense for copy and paste, and was also made to work with find.