Bug 52676

Summary: Stop instantiating legacy editing positions in InsertTextCommand, MoveSelectionCommand, ReplaceSelectionCommand, SelectionController, SpellChecker, TypingCommand, and markup.cpp
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: HTML EditingAssignee: Ryosuke Niwa <rniwa>
Status: RESOLVED FIXED    
Severity: Enhancement CC: darin, eric, ojan, tony
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on:    
Bug Blocks: 52099    
Attachments:
Description Flags
cleanup
none
cleanup eric: review+

Ryosuke Niwa
Reported 2011-01-18 15:52:52 PST
This is a cleanup
Attachments
cleanup (19.98 KB, patch)
2011-01-18 15:55 PST, Ryosuke Niwa 		no flags
DetailsFormatted DiffDiff
cleanup (20.03 KB, patch)
2011-01-18 15:57 PST, Ryosuke Niwa 		eric: review+
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Ryosuke Niwa
Comment 1 2011-01-18 15:55:58 PST
Created attachment 79345 [details] cleanup
Ryosuke Niwa
Comment 2 2011-01-18 15:57:07 PST
Created attachment 79347 [details] cleanup
Ryosuke Niwa
Comment 3 2011-01-24 08:31:25 PST
Could someone review my patch?
Eric Seidel (no email)
Comment 4 2011-01-24 12:03:10 PST
Comment on attachment 79347 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=79347&action=review Seems sane. Please see comments. > Source/WebCore/editing/InsertTextCommand.cpp:175 > + endPosition = Position(textNode, offset + text.length(), Position::PositionIsOffsetInAnchor); This seems like it will be off the end of the node? > Source/WebCore/editing/MoveSelectionCommand.cpp:50 > + if (pos.anchorType() == Position::PositionIsOffsetInAnchor && selectionEnd.anchorType() == Position::PositionIsOffsetInAnchor We need a less cumbersome way to write this check. > Source/WebCore/editing/TypingCommand.cpp:493 > + setEndingSelection(VisibleSelection(positionAfterNode(table), endingSelection().start(), DOWNSTREAM)); After node?
Ryosuke Niwa
Comment 5 2011-01-24 12:25:46 PST
Comment on attachment 79347 [details] cleanup View in context: https://bugs.webkit.org/attachment.cgi?id=79347&action=review >> Source/WebCore/editing/InsertTextCommand.cpp:175 >> + endPosition = Position(textNode, offset + text.length(), Position::PositionIsOffsetInAnchor); > > This seems like it will be off the end of the node? It won't be because we're inserting text at offset. So endPosition is basically the position immediately after the text we just inserted. >> Source/WebCore/editing/MoveSelectionCommand.cpp:50 >> + if (pos.anchorType() == Position::PositionIsOffsetInAnchor && selectionEnd.anchorType() == Position::PositionIsOffsetInAnchor > > We need a less cumbersome way to write this check. I totally agree but we can probably do that kind of cleanup after we're done with the first phase of cleanup.
Ryosuke Niwa
Comment 6 2011-01-24 18:22:04 PST
Committed r76560: <http://trac.webkit.org/changeset/76560>
Note You need to log in before you can comment on or make changes to this bug.