This is similar to the bug 128478. computeSelectionStart() and computeSelectionStart() in HTMLTextFormControlElement use indexForVisiblePosition to compute start and end indexes but we shouldn't need to since we only have br elements and text nodes inside the shadow DOM of inputs and text area elements. We should be able to traverse through the shadow DOM with regular Position instead of VisiblePosition.
Created attachment 224295 [details] Fixes the bug
Comment on attachment 224295 [details] Fixes the bug View in context: https://bugs.webkit.org/attachment.cgi?id=224295&action=review > Source/WebCore/html/HTMLTextFormControlElement.cpp:333 > + int index = indexForPosition(position.deepEquivalent()); Why int rather than unsigned? > Source/WebCore/html/HTMLTextFormControlElement.cpp:601 > + index += std::min(length, static_cast<unsigned>(passedPosition.offsetInContainerNode())); Instead of static_cast<unsigned>, I suggest writing std::min<unsigned>. > Source/WebCore/html/HTMLTextFormControlElement.cpp:616 > +#ifndef NDEBUG > + VisiblePosition visiblePosition = passedPosition; > + unsigned indexComputedByVisiblePosition = 0; > + if (visiblePosition.isNotNull()) > + indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(innerText, visiblePosition, false /* forSelectionPreservation */); > + ASSERT(index == indexComputedByVisiblePosition); > +#endif I think an assertion like this should be written with a helper function so it doesn’t need to be inside an #if. Also can consider ASSERT_DISABLED instead of NDEBUG.
(In reply to comment #2) > (From update of attachment 224295 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=224295&action=review > > > Source/WebCore/html/HTMLTextFormControlElement.cpp:333 > > + int index = indexForPosition(position.deepEquivalent()); > > Why int rather than unsigned? Will fix. > > Source/WebCore/html/HTMLTextFormControlElement.cpp:601 > > + index += std::min(length, static_cast<unsigned>(passedPosition.offsetInContainerNode())); > > Instead of static_cast<unsigned>, I suggest writing std::min<unsigned>. Will fix. > > Source/WebCore/html/HTMLTextFormControlElement.cpp:616 > > +#ifndef NDEBUG > > + VisiblePosition visiblePosition = passedPosition; > > + unsigned indexComputedByVisiblePosition = 0; > > + if (visiblePosition.isNotNull()) > > + indexComputedByVisiblePosition = WebCore::indexForVisiblePosition(innerText, visiblePosition, false /* forSelectionPreservation */); > > + ASSERT(index == indexComputedByVisiblePosition); > > +#endif > > I think an assertion like this should be written with a helper function so it doesn’t need to be inside an #if. Also can consider ASSERT_DISABLED instead of NDEBUG. I'd like to be able to see indexComputedByVisiblePosition and visiblePosition for debugging purposes when the assertion is hit. If I put the computation inside a helper function, it would be tricky to get those objects/values in the debugger. But I'd use ASSERT_DISABLED.
Committed r164180: <http://trac.webkit.org/changeset/164180>