Bug 66014

Summary:

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

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

cfleizach, darin, enrica, eric, tony, webkit.review.bot

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Bug Depends on:

Bug Blocks:

52099

Attachments:

Description	Flags
refactoring	none
fixed build failure	none
Made Position::rootEditableElement() const	none

Ryosuke Niwa

Reported 2011-08-10 14:48:39 PDT

Some low hanging fruits here.

Attachments
refactoring (7.09 KB, patch) 2011-08-10 14:58 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
fixed build failure (7.29 KB, patch) 2011-08-10 15:18 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Made Position::rootEditableElement() const (7.29 KB, patch) 2011-08-16 16:01 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Ryosuke Niwa

Comment 1 2011-08-10 14:58:29 PDT

Created attachment 103538 [details] refactoring

Gyuyoung Kim

Comment 2 2011-08-10 15:04:49 PDT

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 3 2011-08-10 15:06:58 PDT

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

Ryosuke Niwa

Comment 4 2011-08-10 15:18:14 PDT

Created attachment 103539 [details] fixed build failure

Hajime Morrita

Comment 5 2011-08-15 21:17:12 PDT

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

Comment 6 2011-08-16 16:01:41 PDT

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

Hajime Morrita

Comment 7 2011-08-16 23:12:47 PDT

Comment on attachment 104112 [details] Made Position::rootEditableElement() const OK once all bots pass.

Ryosuke Niwa

Comment 8 2011-08-16 23:33:46 PDT

Thanks for the review!

WebKit Review Bot

Comment 9 2011-08-17 03:52:15 PDT

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

Comment 10 2011-08-17 03:52:20 PDT

All reviewed patches have been landed. Closing bug.

Darin Adler

Comment 11 2011-08-17 10:24:02 PDT

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 12 2011-08-17 10:25:55 PDT

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

Comment 13 2011-08-17 10:27:49 PDT

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&.

Note You need to log in before you can comment on or make changes to this bug.