These four tests are skipped by chromium currently, we should finish the implementation and make these tests pass: BUGCR10322 SKIP : accessibility/textarea-insertion-point-line-number.html = TEXT BUGCR10322 SKIP : accessibility/textarea-line-for-index.html = TEXT BUGCR10322 SKIP : accessibility/textarea-selected-text-range.html = TEXT BUGCR10322 SKIP : accessibility/textbox-role-reports-selection.html = TEXT
Created attachment 164974 [details] Patch
Created attachment 165146 [details] Rebase
Please wait for approval from abarth@webkit.org, dglazkov@chromium.org, fishd@chromium.org, jamesr@chromium.org or tkent@chromium.org before submitting, as this patch contains changes to the Chromium public API. See also https://trac.webkit.org/wiki/ChromiumWebKitAPI.
Chris, this should be ready for review when you have a chance - thanks.
Comment on attachment 165146 [details] Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=165146&action=review > Source/WebCore/ChangeLog:9 > + Adds a method to get all line breaks at once, so retrieving all > + line breaks isn't an O(n^2) operation for Chromium. Is it O(N^2) for other platforms? :)
(In reply to comment #5) > (From update of attachment 165146 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=165146&action=review > > > Source/WebCore/ChangeLog:9 > > + Adds a method to get all line breaks at once, so retrieving all > > + line breaks isn't an O(n^2) operation for Chromium. > > Is it O(N^2) for other platforms? :) Good question. Short answer: no, this optimization is only needed for Chromium. Longer answer: In the Mac port of WebKit, with VoiceOver, the line break functions are called as needed when a user is interacting with something like a textarea. Each call to get the line number is O(N) in the number of lines, but there are only a constant number of such calls per character typed. The GTK port is the same. In Chromium, we cache the information about the line breaks in the browser process so that screen reader queries about line breaks can be satisfied from the cache without waiting on an IPC to the renderer - so we have to pay the penalty of computing all of the line breaks each time the textbox is modified.
Comment on attachment 165146 [details] Rebase View in context: https://bugs.webkit.org/attachment.cgi?id=165146&action=review mostly minor nits. i think this looks ok once those are addressed > Source/WebCore/accessibility/AccessibilityObject.h:637 > + virtual void allLineBreaks(Vector<int>&) const { } i think "all" is implicit in the name. Just naming it "lineBreaks" feels like it's enough > Source/WebKit/chromium/src/WebAccessibilityObject.cpp:770 > + for (size_t i = 0; i< lineBreaksVector.size(); i++) i would move .size() call out of for loop so it doesn't have to be called each iteration > Tools/DumpRenderTree/chromium/TestRunner/AccessibilityUIElementChromium.cpp:647 > + while (line < static_cast<int>(lineBreaks.size()) && lineBreaks[line] <= index) move .size() call out of loop
Created attachment 165637 [details] Patch for landing
Comment on attachment 165637 [details] Patch for landing Clearing flags on attachment: 165637 Committed r129527: <http://trac.webkit.org/changeset/129527>
All reviewed patches have been landed. Closing bug.