[webkit-reviews] review denied: [Bug 48037] Triple click does not select whole line for mixed contenteditable regions : [Attachment 71850] Patch

[bugzilla-daemon at webkit.org]

Ojan Vafai <ojan at chromium.org> has denied kalman at chromium.org's request for
review:
Bug 48037: Triple click does not select whole line for mixed contenteditable
regions
https://bugs.webkit.org/show_bug.cgi?id=48037

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

------- Additional Comments from Ojan Vafai <ojan at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=71850&action=review

I agree that we should be able to select non-editable regions inside an
editable region using these APIs. The user can do it using the mouse. The
important thing is to be careful to make sure we don't select part of a
non-editable region. It needs to be atomic. Can you add to your test the cases
where the non-editable span is at the beginning/end of the paragraph?

r- for the test case

FWIW, I agree that script-tests are unfortunate. Feel free to fix
https://bugs.webkit.org/show_bug.cgi?id=48344. :)

>>>
LayoutTests/editing/selection/script-tests/triple-click-across-editability-boun
daries.js:70
>>> +	 //testTripleClick("bar baz rab", "middle", test3);
>> 
>> This is interesting, possibly a bug in
VisibleSelection::adjustSelectionToAvoidCrossingEditingBoundaries ?
> 
> This is a quite serious bug.	We should fix this bug before checking in this
patch.

It's not clear to me what the actual result here is. The standard behavior here
is to put a FIXME, have it spit out the FAIL line and check in the expectations
with the FAIL line. That's better that commented out code because it keeps the
codepath tested (e.g. if someone ends up changing this in the future).