Instead of keeping old constructor, we should make the way to instantiate a legacy editing position more explicit so that people will pay more attention to the deprecated code.
This step is also necessary to add a new Position constructor that takes Text* and offset.
Created attachment 97924 [details] cleanup; likely to fail build on various ports
Comment on attachment 97924 [details] cleanup; likely to fail build on various ports Attachment 97924 [details] did not pass gtk-ews (gtk): Output: http://queues.webkit.org/results/8909844
(In reply to comment #3) > (From update of attachment 97924 [details]) > Attachment 97924 [details] did not pass gtk-ews (gtk): > Output: http://queues.webkit.org/results/8909844 http://trac.webkit.org/browser/trunk/Source/WebCore/accessibility/gtk/AXObjectCacheAtk.cpp: Range::create(node->document(), Position(node->parentNode(), 0), Position(node, 0)); This is terrible :(
Comment on attachment 97924 [details] cleanup; likely to fail build on various ports View in context: https://bugs.webkit.org/attachment.cgi?id=97924&action=review > Source/WebCore/dom/Position.cpp:272 > + return createDeprecatedLegacyEditingPosition(n, uncheckedPreviousOffsetForBackwardDeletion(n, o)); Huh? Why not just createLegacyEditingPosition? The deprecated seems redundant here.
(In reply to comment #5) > (From update of attachment 97924 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=97924&action=review > > > Source/WebCore/dom/Position.cpp:272 > > + return createDeprecatedLegacyEditingPosition(n, uncheckedPreviousOffsetForBackwardDeletion(n, o)); > > Huh? Why not just createLegacyEditingPosition? The deprecated seems redundant here. Sure. I can do that.
Comment on attachment 97924 [details] cleanup; likely to fail build on various ports Looks like the only failure EWS found is: dom/Position.h: In member function ‘void WebCore::AXObjectCache::nodeTextChangePlatformNotification(WebCore::AccessibilityObject*, WebCore::AXObjectCache::AXTextChange, unsigned int, unsigned int)’: dom/Position.h:73:9: error: ‘WebCore::Position::LegacyEditingOffset::LegacyEditingOffset(int)’ is private accessibility/gtk/AXObjectCacheAtk.cpp:190:90: error: within this context dom/Position.h:73:9: error: ‘WebCore::Position::LegacyEditingOffset::LegacyEditingOffset(int)’ is private accessibility/gtk/AXObjectCacheAtk.cpp:190:109: error: within this context
(In reply to comment #7) > (From update of attachment 97924 [details]) > Looks like the only failure EWS found is: > > dom/Position.h: In member function ‘void WebCore::AXObjectCache::nodeTextChangePlatformNotification(WebCore::AccessibilityObject*, WebCore::AXObjectCache::AXTextChange, unsigned int, unsigned int)’: > dom/Position.h:73:9: error: ‘WebCore::Position::LegacyEditingOffset::LegacyEditingOffset(int)’ is private > accessibility/gtk/AXObjectCacheAtk.cpp:190:90: error: within this context > dom/Position.h:73:9: error: ‘WebCore::Position::LegacyEditingOffset::LegacyEditingOffset(int)’ is private > accessibility/gtk/AXObjectCacheAtk.cpp:190:109: error: within this context Right. See comment #4. AXObjectCacheAtk.cpp is instantiating a Range with two legacy editing positions.
Created attachment 98002 [details] createDeprecatedLegacyEditingPosition to createLegacyEditingPosition; fixed gtk build
Ping reviewers
Comment on attachment 98002 [details] createDeprecatedLegacyEditingPosition to createLegacyEditingPosition; fixed gtk build View in context: https://bugs.webkit.org/attachment.cgi?id=98002&action=review > Source/WebCore/dom/Position.h:70 > + int value() { return m_offset; } Maybe make this const? > Source/WebCore/dom/Position.h:73 > + LegacyEditingOffset(int offset) : m_offset(offset) { } Maybe make this explicit too?
Thanks for the review. (In reply to comment #11) > (From update of attachment 98002 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=98002&action=review > > > Source/WebCore/dom/Position.h:70 > > + int value() { return m_offset; } > > Maybe make this const? > > > Source/WebCore/dom/Position.h:73 > > + LegacyEditingOffset(int offset) : m_offset(offset) { } > > Maybe make this explicit too? Will fix.
Committed r89440: <http://trac.webkit.org/changeset/89440>