[webkit-reviews] review denied: [Bug 27523] Don't convert soft hyphens in selections into spaces : [Attachment 33222] patch

[bugzilla-daemon at webkit.org]

Eric Seidel <eric at webkit.org> has denied Adam Langley <agl at chromium.org>'s
request for review:
Bug 27523: Don't convert soft hyphens in selections into spaces
https://bugs.webkit.org/show_bug.cgi?id=27523

Attachment 33222: patch
https://bugs.webkit.org/attachment.cgi?id=33222&action=review

------- Additional Comments from Eric Seidel <eric at webkit.org>
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.