Created attachment 136151 [details] test case +++ This bug was initially created as a clone of Bug #73431 +++ Steps to reproduce: 1. View the attached test case in Epiphany 2. Resize the window so that the text wraps 3. Enable caret browsing 4. Launch the attached test script in a terminal 5. Arrow Up and Down amongst the lines of text Expected results: The test script would always print the correct text for the current line. Actual results: The test script fails to present the correct text for the second list item.
Created attachment 136152 [details] test script
Created attachment 136264 [details] Another test case An additional get_text_at_offset() failure/test case. (Not sure if it's a variant of the same bug or a different bug.)
Created attachment 158452 [details] proposed patch with updated unit test
Comment on attachment 158452 [details] proposed patch with updated unit test View in context: https://bugs.webkit.org/attachment.cgi?id=158452&action=review Thanks for the patch. It looks good to me with the exception of the missing comments in the ChangeLog. You should still ask for an official review, though > Source/WebCore/ChangeLog:11 > + (textForRenderer): A brief description of the change for textForRenderer would be nice here. > Source/WebCore/accessibility/gtk/WebKitAccessibleInterfaceText.cpp:82 > + if (object->isReplaced() && !object->isListMarker()) Oh! It indeed looks like a mistake in the code (and not coherent withthe comment). Good catch (/me runs and hides) > Source/WebKit/gtk/ChangeLog:11 > + (testWebkitAtkGetTextAtOffsetWithSpecialCharacters): Something like "Updated unit test" would be nice here
(In reply to comment #4) > (From update of attachment 158452 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=158452&action=review > > Thanks for the patch. It looks good to me with the exception of the missing comments in the ChangeLog. Thanks for the review! :) Perhaps I should have coffee first and then respond, but.... What missing comments? Looking at: https://bugs.webkit.org/attachment.cgi?id=158452&action=prettypatch, I see: Source/WebCore/ChangeLog 8 Be sure the list item marker to be ignored is actually a list item marker prior to ignoring it. Source/WebKit/gtk/ChangeLog 8 Include a paragraph in a list item when testing atk_text_get_text_at_offset().
Adding Chris to CC for review. (It's a one-line bug fix.)
(In reply to comment #5) > (In reply to comment #4) > > (From update of attachment 158452 [details] [details]) > > View in context: https://bugs.webkit.org/attachment.cgi?id=158452&action=review > > > > Thanks for the patch. It looks good to me with the exception of the missing comments in the ChangeLog. > > Thanks for the review! :) Perhaps I should have coffee first and then respond, but.... What missing comments? Looks like I should be the one drinking coffee first :-). I meant the missing comments _per-function_, but it's true that in this case it maybe makes not much sense since it would be duplicating the stuff in the general description. Anyway, in my defense I must say I should not be here today as it's a bank day and so I'm officially not working :).
Comment on attachment 158452 [details] proposed patch with updated unit test View in context: https://bugs.webkit.org/attachment.cgi?id=158452&action=review > Source/WebCore/ChangeLog:8 > + Be sure the list item marker to be ignored is actually a list item marker prior to ignoring it. i think this comment is a little hard to follow Maybe something like "Fix a logic error when checking if an object is a list marker" >> Source/WebKit/gtk/ChangeLog:11 >> + (testWebkitAtkGetTextAtOffsetWithSpecialCharacters): > > Something like "Updated unit test" would be nice here Agreed "Updated unit test to include a paragraph in a list ..."
Created attachment 158583 [details] Updated patch to address review comments
Comment on attachment 158583 [details] Updated patch to address review comments Clearing flags on attachment: 158583 Committed r125685: <http://trac.webkit.org/changeset/125685>
All reviewed patches have been landed. Closing bug.