Bug 65221

Summary:

Add RenderedPosition

Product:

WebKit

Reporter:

Ryosuke Niwa <rniwa>

Component:

HTML Editing

Assignee:

Ryosuke Niwa <rniwa>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

adele, cfleizach, darin, enrica, eric, leviw, ojan, tony, vanlam, webkit.review.bot, xji

Priority:

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
adds the skeleton	none
adds the skeleton; made some constructors explicit	none
Removed unnecessary constructor	none

Ryosuke Niwa

Reported 2011-07-26 17:15:33 PDT

As proposed during the contributor's meeting, we should add RenderedPosition. I wasn't sure if this will be a good refactoring but patches for the bug 60910 and my recent works on hit testing code convinced me that this will be useful. https://docs.google.com/document/d/1vzzi_wQHO0GtLnu-1IYutUKbcpAYlHqZwQlwbfLuowM/edit?hl=en_US&authkey=CLOa9cgN

Attachments
adds the skeleton (16.47 KB, patch) 2011-07-26 17:19 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
adds the skeleton; made some constructors explicit (16.49 KB, patch) 2011-07-26 17:30 PDT, Ryosuke Niwa	no flags	Details Formatted Diff Diff
Removed unnecessary constructor (18.04 KB, patch) 2011-08-01 10:46 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-07-26 17:19:41 PDT

Created attachment 102077 [details] adds the skeleton

Ryosuke Niwa

Comment 2 2011-07-26 17:25:53 PDT

Comment on attachment 102077 [details] adds the skeleton View in context: https://bugs.webkit.org/attachment.cgi?id=102077&action=review > Source/WebCore/editing/RenderedPosition.h:47 > + RenderedPosition(const Position&, EAffinity); > + RenderedPosition(const VisiblePosition&); I should make these explicit.

Ryosuke Niwa

Comment 3 2011-07-26 17:30:15 PDT

Created attachment 102081 [details] adds the skeleton; made some constructors explicit

Ryosuke Niwa

Comment 4 2011-07-30 10:40:27 PDT

Ping reviewers

Hajime Morrita

Comment 5 2011-07-31 23:24:12 PDT

Comment on attachment 102081 [details] adds the skeleton; made some constructors explicit View in context: https://bugs.webkit.org/attachment.cgi?id=102081&action=review > Source/WebCore/editing/RenderedPosition.h:53 > + RenderObject* m_renderer; Do we need m_renderer? It loks we can retrieve this from m_inlineBox.

Ryosuke Niwa

Comment 6 2011-07-31 23:28:59 PDT

Comment on attachment 102081 [details] adds the skeleton; made some constructors explicit View in context: https://bugs.webkit.org/attachment.cgi?id=102081&action=review >> Source/WebCore/editing/RenderedPosition.h:53 >> + RenderObject* m_renderer; > > Do we need m_renderer? It loks we can retrieve this from m_inlineBox. We don't in the current version but we will in the future where we'll have an empty visible block nodes. e.g. <div></div><div></div>

Hajime Morrita

Comment 7 2011-07-31 23:45:16 PDT

Comment on attachment 102081 [details] adds the skeleton; made some constructors explicit View in context: https://bugs.webkit.org/attachment.cgi?id=102081&action=review > Source/WebCore/editing/RenderedPosition.h:47 > + explicit RenderedPosition(const VisiblePosition&); Do we need both? Current code covers only the second version. >>> Source/WebCore/editing/RenderedPosition.h:53 >>> + RenderObject* m_renderer; >> >> Do we need m_renderer? It loks we can retrieve this from m_inlineBox. > > We don't in the current version but we will in the future where we'll have an empty visible block nodes. e.g. <div></div><div></div> That makes sense. > Source/WebCore/editing/visible_units.cpp:327 > +static inline RootInlineBox* rootBoxForLine(const VisiblePosition& currentPosition) It looks we no longer need this function. RenderedPosition(currentPosition).rootBox() looks concise and descriptive enough.

Ryosuke Niwa

Comment 8 2011-08-01 10:46:31 PDT

Created attachment 102535 [details] Removed unnecessary constructor

Ryosuke Niwa

Comment 9 2011-08-02 12:28:16 PDT

Ping reviewers.

Ryosuke Niwa

Comment 10 2011-08-03 08:29:30 PDT

Thanks for the review!

WebKit Review Bot

Comment 11 2011-08-03 09:27:50 PDT

Comment on attachment 102535 [details] Removed unnecessary constructor Clearing flags on attachment: 102535 Committed r92286: <http://trac.webkit.org/changeset/92286>

WebKit Review Bot

Comment 12 2011-08-03 09:27:57 PDT

All reviewed patches have been landed. Closing bug.

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