Bug 135233 - Member name collision in RenderView and SelectionSubtreeRoot
Summary: Member name collision in RenderView and SelectionSubtreeRoot
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Radu Stavila
URL:
Keywords: AdobeTracked
Depends on:
Blocks:
 
Reported: 2014-07-24 02:39 PDT by Radu Stavila
Modified: 2014-07-30 09:23 PDT (History)
9 users (show)

See Also:


Attachments
Patch for renaming selection members - NOT FOR LANDING (4.87 KB, patch)
2014-07-24 03:04 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch (5.52 KB, patch)
2014-07-30 04:11 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff
Patch integrating reviewer feedback (5.52 KB, patch)
2014-07-30 06:38 PDT, Radu Stavila
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Radu Stavila 2014-07-24 02:39:29 PDT
SelectionSubtreeRoot members:

    RenderObject* m_selectionStart;
    int m_selectionStartPos;
    RenderObject* m_selectionEnd;
    int m_selectionEndPos;


RenderView members:

    RenderObject* m_selectionStart;
    RenderObject* m_selectionEnd;
    int m_selectionStartPos;
    int m_selectionEndPos;


Because RenderView inherits SelectionSubtreeRoot, this presents a problem. Furthermore, the getter/setter methods also have identical names (selectionStart(), setSelectionStart(...) etc). 

Example: in RenderView.cpp, setting the start/end selection members to some values. Then calling selectionStartEndPositions(...) which retrieves those values. Because this method is inside SelectionSubtreeRoot, it will return the version of the selection members from SelectionSubtreeRoot, not the ones you would normally expect it to. This has great potential for creating confusion and hard-to-track issues.
Comment 1 Radu Stavila 2014-07-24 03:04:21 PDT
Created attachment 235420 [details]
Patch for renaming selection members - NOT FOR LANDING

The attached patch renames the members inside RenderView but causes fast/dom/HTMLObjectElement/beforeload-set-text-crash.xhtml to crash.
Comment 2 Radu Stavila 2014-07-28 06:07:03 PDT
Update: the crash was not caused by the rename. After the initial patch lands (https://bugs.webkit.org/show_bug.cgi?id=134303) I will try to re-land this.
Comment 3 Radu Stavila 2014-07-30 04:11:46 PDT
Created attachment 235740 [details]
Patch
Comment 4 Mihnea Ovidenie 2014-07-30 04:43:34 PDT
Comment on attachment 235740 [details]
Patch

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

> Source/WebCore/rendering/RenderView.h:92
> +    RenderObject* selectionUnsplitStart() const { return m_unsplitSelectionStart; }

Any reason why the function is named selectionUnsplitStart and the member is m_unsplitSelectionStart?
Comment 5 Radu Stavila 2014-07-30 06:38:27 PDT
Created attachment 235743 [details]
Patch integrating reviewer feedback
Comment 6 Mihnea Ovidenie 2014-07-30 07:02:23 PDT
Comment on attachment 235743 [details]
Patch integrating reviewer feedback

r=me
Comment 7 WebKit Commit Bot 2014-07-30 09:23:21 PDT
Comment on attachment 235743 [details]
Patch integrating reviewer feedback

Clearing flags on attachment: 235743

Committed r171797: <http://trac.webkit.org/changeset/171797>
Comment 8 WebKit Commit Bot 2014-07-30 09:23:25 PDT
All reviewed patches have been landed.  Closing bug.