Merge nextRootInlineBox with nextLinePosition
Created attachment 132721 [details] Patch
Comment on attachment 132721 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=132721&action=review > Source/WebCore/ChangeLog:9 > + Refactroed nextLinePosition and previousLinePosition to use nextRootInlineBox and previousRootInlineBox. > + Can you give me a bit more high-level overview of what's going on here? What did these fucntions used to do? Why are we merging them, etc...
(In reply to comment #2) > (From update of attachment 132721 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=132721&action=review > > > Source/WebCore/ChangeLog:9 > > + Refactroed nextLinePosition and previousLinePosition to use nextRootInlineBox and previousRootInlineBox. > > + > > Can you give me a bit more high-level overview of what's going on here? What did these fucntions used to do? Why are we merging them, etc... nextRootInlineBox and previousRootInlineBox look for previous or next root inline boxes across renderers, and nextLinePosition and previousLinePosition had duplicated code in them. I had asked xji (who recently added nextRootInlineBox and previousRootInlineBox) to refactor them later but it turned out that I needed this refactoring in order to fix the bug 81490.
Comment on attachment 132721 [details] Patch Attachment 132721 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/12000071 New failing tests: editing/selection/3690703-2.html editing/selection/3690703.html
Odd. This failure doesn't reproduce on Windows port either :(
Diffs from chromium-linux: --- /src/chrome/src/webkit/Release/layout-test-results/editing/selection/3690703-2-expected.txt +++ /src/chrome/src/webkit/Release/layout-test-results/editing/selection/3690703-2-actual.txt @@ -147,5 +147,5 @@ RenderInline {FONT} at (0,0) size 60x12 RenderText {#text} at (362,0) size 60x12 text run at (362,0) width 60: "\x{A9}2005 Google" -selection start: position 0 of child 3 {INPUT} of child 1 {TD} of child 0 {TR} of child 0 {TBODY} of child 2 {TABLE} of child 4 {DIV} of child 0 {CENTER} of body -selection end: position 1 of child 2 {BR} of child 0 {FONT} of child 2 {TD} of child 0 {TR} of child 0 {TBODY} of child 2 {TABLE} of child 4 {DIV} of child 0 {CENTER} of body +selection start: position 0 of child 0 {#text} of child 0 {FONT} of child 2 {TD} of child 0 {TR} of child 0 {TBODY} of child 2 {TABLE} of child 4 {DIV} of child 0 {CENTER} of body +selection end: position 1 of child 5 {BR} of child 0 {FONT} of child 2 {TD} of child 0 {TR} of child 0 {TBODY} of child 2 {TABLE} of child 4 {DIV} of child 0 {CENTER} of body --- /src/chrome/src/webkit/Release/layout-test-results/editing/selection/3690703-expected.txt +++ /src/chrome/src/webkit/Release/layout-test-results/editing/selection/3690703-actual.txt @@ -1,6 +1,5 @@ EDITING DELEGATE: shouldBeginEditingInDOMRange:range from 0 of DIV > CENTER > BODY > HTML > #document to 6 of DIV > CENTER > BODY > HTML > #document EDITING DELEGATE: webViewDidBeginEditing:WebViewDidBeginEditingNotification -EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification EDITING DELEGATE: webViewDidChangeSelection:WebViewDidChangeSelectionNotification
I don't quite understand what's causing the failure. As far as I know, there should be no difference between Mac and Chromium Linux when moving or extending selection via Selection.modify. Unfortunately, it seems intractable to debug this regression without access to cr-linux, so I'll give up on this patch.
Created attachment 141880 [details] Less intrusive patch
Xiaomei must have cleaned up the code there since this patch was much easier to write.
Comment on attachment 141880 [details] Less intrusive patch Ugh... that's because I didn't complete the second half.
Created attachment 141901 [details] Deleted more code but considerably more complicated patch
Comment on attachment 141901 [details] Deleted more code but considerably more complicated patch View in context: https://bugs.webkit.org/attachment.cgi?id=141901&action=review The change looks good. > Source/WebCore/ChangeLog:9 > + to share the code. Moved out the nullity check of startBox and extraction of the renderer's node from should be: "and extracted the renderer's node..."
Committed r117359: <http://trac.webkit.org/changeset/117359>