Summary: | Move RangeBoundaryPoint off of Position | ||
---|---|---|---|
Product: | WebKit | Reporter: | Eric Seidel (no email) <eric> |
Component: | HTML Editing | Assignee: | Nobody <webkit-unassigned> |
Status: | RESOLVED FIXED | ||
Severity: | Normal | CC: | darin, jparent, justin.garcia, ojan |
Priority: | P2 | ||
Version: | 528+ (Nightly build) | ||
Hardware: | Mac | ||
OS: | OS X 10.5 | ||
Bug Depends on: | 24763 | ||
Bug Blocks: | |||
Attachments: |
Description
Eric Seidel (no email)
2009-03-31 15:17:57 PDT
Created attachment 29136 [details]
Add containerNode(), computeOffsetInContainerNode(), nodeBeforePosition(), nodeAfterPosition()
WebCore/ChangeLog | 21 +++++++++++++
WebCore/dom/Position.cpp | 76 ++++++++++++++++++++++++++++++++++++++++++++++
WebCore/dom/Position.h | 17 ++++++++++
3 files changed, 114 insertions(+), 0 deletions(-)
Created attachment 29137 [details]
Move RangeBoundaryPoint off of Position, per Darin's suggestion
WebCore/ChangeLog | 33 +++++++++++
WebCore/dom/Position.h | 16 +++--
WebCore/dom/Range.cpp | 8 +-
WebCore/dom/Range.h | 4 +-
WebCore/dom/RangeBoundaryPoint.h | 114 +++++++++++++++++++++-----------------
5 files changed, 113 insertions(+), 62 deletions(-)
All layout tests pass. Comment on attachment 29137 [details]
Move RangeBoundaryPoint off of Position, per Darin's suggestion
Looks good.
r=me
Do we need RangeBoundaryPoint::toPosition()? I think RangeBoundaryPoint may be a lower level class than Position, so conversion to a Position might insteadCan we get rid of Range::startPosition and Range::endPosition?
Comment on attachment 29136 [details]
Add containerNode(), computeOffsetInContainerNode(), nodeBeforePosition(), nodeAfterPosition()
Uploading an improved patch in a sec.
Created attachment 29151 [details]
Add containerNode(), computeOffsetInContainerNode(), nodeBeforePosition(), nodeAfterPosition()
WebCore/ChangeLog | 21 +++++++++++
WebCore/dom/Position.cpp | 85 ++++++++++++++++++++++++++++++++++++++++++++++
WebCore/dom/Position.h | 17 +++++++++
3 files changed, 123 insertions(+), 0 deletions(-)
Created attachment 29152 [details]
Move RangeBoundaryPoint off of Position, per Darin's suggestion
WebCore/ChangeLog | 33 +++++++++++
WebCore/dom/Position.h | 16 +++--
WebCore/dom/Range.cpp | 8 +-
WebCore/dom/Range.h | 4 +-
WebCore/dom/RangeBoundaryPoint.h | 114 +++++++++++++++++++++-----------------
5 files changed, 113 insertions(+), 62 deletions(-)
Created attachment 29153 [details]
Make Position::m_offset private and fix callers to use either deprecatedEditingOffset() or offsetInContainer()
LayoutTests/ChangeLog | 11 ++
.../assert-on-range-creation-expected.txt | 1 +
.../execCommand/assert-on-range-creation.html | 12 ++
WebCore/ChangeLog | 140 ++++++++++++++++++++
WebCore/WebCore.base.exp | 2 +
WebCore/dom/Position.h | 19 ++-
WebCore/dom/PositionIterator.h | 4 +-
WebCore/dom/Range.cpp | 19 +++-
WebCore/dom/Range.h | 4 +
WebCore/editing/ApplyStyleCommand.cpp | 90 +++++++------
WebCore/editing/BreakBlockquoteCommand.cpp | 8 +-
WebCore/editing/CompositeEditCommand.cpp | 34 +++---
WebCore/editing/DeleteSelectionCommand.cpp | 47 ++++----
WebCore/editing/Editor.cpp | 14 +-
WebCore/editing/InsertLineBreakCommand.cpp | 6 +-
.../editing/InsertParagraphSeparatorCommand.cpp | 14 +-
WebCore/editing/InsertTextCommand.cpp | 12 +-
WebCore/editing/MoveSelectionCommand.cpp | 6 +-
WebCore/editing/ReplaceSelectionCommand.cpp | 2 +-
WebCore/editing/SelectionController.cpp | 4 +-
WebCore/editing/TextIterator.cpp | 2 +-
WebCore/editing/TypingCommand.cpp | 10 +-
WebCore/editing/VisiblePosition.cpp | 46 +++----
WebCore/editing/VisibleSelection.cpp | 12 +-
WebCore/editing/htmlediting.cpp | 20 ++-
WebCore/editing/visible_units.cpp | 40 +++---
WebCore/page/AccessibilityObject.cpp | 2 +-
WebCore/page/AccessibilityRenderObject.cpp | 4 +-
WebCore/page/DOMSelection.cpp | 16 +-
WebCore/page/EventHandler.cpp | 2 +-
WebCore/page/Frame.cpp | 2 +-
WebCore/page/mac/AccessibilityObjectWrapper.mm | 4 +-
WebCore/rendering/RenderTextControl.cpp | 2 +-
WebCore/rendering/RenderTreeAsText.cpp | 6 +-
WebKit/mac/ChangeLog | 11 ++
WebKit/mac/WebView/WebFrame.mm | 7 +-
36 files changed, 424 insertions(+), 211 deletions(-)
Comment on attachment 29152 [details]
Move RangeBoundaryPoint off of Position, per Darin's suggestion
Uploaded again by accident.
Why are these other patches in this bug? Given the title, doesn’t make sense. I did these patches all as one series. I'm happy to put the other two patches on separate bugs. The "add containerNode()..." patch is actually the first patch. But again, I'm happy to put them on other bugs if that would make things easier. (In reply to comment #11) > I did these patches all as one series. I'm happy to put the other two patches > on separate bugs. The "add containerNode()..." patch is actually the first > patch. But again, I'm happy to put them on other bugs if that would make > things easier. If you want to do this all in one bug report, that's fine. But please give the bug report a title that corresponds to the patches. These patches aren’t about "move RangeBoundaryPoint off of Position", and I have trouble understanding their context so I can give them a good review without a bug title explaining the high level task. Comment on attachment 29151 [details]
Add containerNode(), computeOffsetInContainerNode(), nodeBeforePosition(), nodeAfterPosition()
Adding these to other bugs per Darin's suggestion.
Comment on attachment 29153 [details] Make Position::m_offset private and fix callers to use either deprecatedEditingOffset() or offsetInContainer() Moving this patch to a new bug (bug 25056) per Darin's suggestion. I will land this once the first patch from bug 24763 is reviewed (as this patch was created as part of a series of patches and currently depends on that one... although doesn't in the end need to). Thanks for the review darin. Landed as: M WebCore/ChangeLog M WebCore/dom/Position.h M WebCore/dom/Range.cpp M WebCore/dom/Range.h M WebCore/dom/RangeBoundaryPoint.h Committed r42264 |