See title
Created attachment 102988 [details] Proposed fix Renamed greatestValueUnder to greatestOffsetUnder, positionIsInsideBox to positionIsInBoxButNotOnBoundary (to avoid confusion with positionIsInBox, which is just a getInlineBoxAndOffset check). Removed use of invalidOffset as an error value in greatestOffsetUnder and smallestOffsetAbove since semantically it should only be used to check if it makes sense to compare offsets in a single box.
Comment on attachment 102988 [details] Proposed fix View in context: https://bugs.webkit.org/attachment.cgi?id=102988&action=review > Source/WebCore/editing/visible_units.cpp:1644 > + if (index != -1) Probably better to check index < 0. > Source/WebCore/editing/visible_units.cpp:1681 > + if (index != -1) Ditto.
Created attachment 102991 [details] Revised fix Changed comparison index != -1 to index >= 0.
Comment on attachment 102991 [details] Revised fix View in context: https://bugs.webkit.org/attachment.cgi?id=102991&action=review > Source/WebCore/editing/visible_units.cpp:1643 > + : smallestOffsetAbove(offset, blockDirection == RTL, orderedWordBoundaries); Oops, cq- because you forgot to indent here.
Created attachment 102992 [details] Revised fix Indented. Nice catch :)
Comment on attachment 102992 [details] Revised fix Clearing flags on attachment: 102992 Committed r92431: <http://trac.webkit.org/changeset/92431>
All reviewed patches have been landed. Closing bug.
Comment on attachment 102992 [details] Revised fix View in context: https://bugs.webkit.org/attachment.cgi?id=102992&action=review > Source/WebCore/editing/visible_units.cpp:1450 > - return invalidOffset; > + return -1; Replacing a name constant with a magic value of -1 does not seem like a style improvement to me.
Comment on attachment 102992 [details] Revised fix View in context: https://bugs.webkit.org/attachment.cgi?id=102992&action=review >> Source/WebCore/editing/visible_units.cpp:1450 >> + return -1; > > Replacing a name constant with a magic value of -1 does not seem like a style improvement to me. invalidOffset was supposed to be used only for offsets in inline boxes. So this was misused here. We can introduce new const like NotFound instead.