Bug 66014 – Get rid of calls to deprecatedNode and deprecatedEditingOffset in AccessibilityRenderObject.cpp and InsertTextCommand.cpp

• Status: RESOLVED
• Resolution: FIXED
• Component: HTML Editing

[Ryosuke Niwa]

Some low hanging fruits here.

[Ryosuke Niwa]

Created attachment 103538 [details]
refactoring

[Gyuyoung Kim]

Comment on attachment 103538 [details]
refactoring

Attachment 103538 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/9344306

[Early Warning System Bot]

Comment on attachment 103538 [details]
refactoring

Attachment 103538 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/9344307

[Ryosuke Niwa]

Created attachment 103539 [details]
fixed build failure

[Hajime Morrita]

Comment on attachment 103539 [details]
fixed build failure

View in context: https://bugs.webkit.org/attachment.cgi?id=103539&action=review

> Source/WebCore/dom/Position.h:131
> +    Element* rootEditableElement()

const?

> Source/WebCore/dom/Range.h:106
> +    void setStart(const Position&, ExceptionCode&);

You can give ASSERT_NO_EXCEPTION for the default value of the second parameter.

[Ryosuke Niwa]

Created attachment 104112 [details]
Made Position::rootEditableElement() const

[Hajime Morrita]

Comment on attachment 104112 [details]
Made Position::rootEditableElement() const

OK once all bots pass.

[Ryosuke Niwa]

Thanks for the review!

[WebKit Review Bot]

Comment on attachment 104112 [details]
Made Position::rootEditableElement() const

Clearing flags on attachment: 104112

Committed r93199: <http://trac.webkit.org/changeset/93199>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Darin Adler]

Comment on attachment 104112 [details]
Made Position::rootEditableElement() const

View in context: https://bugs.webkit.org/attachment.cgi?id=104112&action=review

> Source/WebCore/dom/Range.cpp:287
> +void Range::setStart(const Position& start, ExceptionCode& ec)
> +{
> +    Position parentAnchored = start.parentAnchoredEquivalent();
> +    setStart(parentAnchored.containerNode(), parentAnchored.offsetInContainerNode(), ec);
> +}
> +
> +void Range::setEnd(const Position& end, ExceptionCode& ec)
> +{
> +    Position parentAnchored = end.parentAnchoredEquivalent();
> +    setStart(parentAnchored.containerNode(), parentAnchored.offsetInContainerNode(), ec);
> +}

Do callers really need the exception capability? Instead could we just have these fail silently or assert?

[Ryosuke Niwa]

Comment on attachment 104112 [details]
Made Position::rootEditableElement() const

View in context: https://bugs.webkit.org/attachment.cgi?id=104112&action=review

>> Source/WebCore/dom/Range.cpp:287
>> +}
> 
> Do callers really need the exception capability? Instead could we just have these fail silently or assert?

We can do that.  Or pass ASSERT_NO_EXCEPTION by default as morrita suggested.

[Darin Adler]

Generally, the ExceptionCode& thing should only be used for actual DOM binding since it’s how the DOM cis specified. Not internal use, because it’s an awkward way to report errors in C++.

ASSERT_NO_EXCEPTION is a fine way to make a function that’s intended for DOM binding easier to use internally. But a function intended for user internally probably should not even use ExceptionCode&.