Bug 53024

Proposed fix: This is a limited fix that only addresses the link between the renderers of :before and :after pseudo elements and their generating element. Currently when renderers are created there is a convention that an anonymous renderer is designated by passing the document as the associated node. At construction time this property (anonymity) is preserved in a bit field and the m_node member stores a pointer to the document node. This stored pointer is redundant from this point on so the idea of this fix is to reuse the location of this pointer (m_node) to store a pointer to the generating node. the m_node is private to RenderObject and is never retrieved directly. The node() accessor used to retrieve this value returns 0 for anonymous renderers, thus the behavior will not be affected by the change. There is already a setNode() accessor to change the value of m_node used by RenderWidget to reset m_node when the RenderWidget is destroyed but its refcount did not reach 0, thus it can't be freed. I propose introducing a new accessor generatingNode() that would return m_node unconditionally, making it possible to obtain thois information for anonymous nodes. Also, I propose introducing after() and before() accessors to retrieve the after and before pseudo elements that may be attached to a renderer, as the algorithm to get these is now private to RenderObjectChildren, and is also needed elsewhere such as DumpRenderTree::LayoutController::getCounterValueForId() and the counter evaluation code.

Created attachment 79952 [details] Proposed Patch

Attachment 79952 [details] did not build on chromium: Build output: http://queues.webkit.org/results/7493327

Created attachment 79956 [details] Proposed Patch. Fixed chromium build issue

Comment on attachment 79956 [details] Proposed Patch. Fixed chromium build issue View in context: https://bugs.webkit.org/attachment.cgi?id=79956&action=review > Source/WebCore/rendering/RenderObject.h:157 > + RenderObject* before() const > + { > + if (const RenderObjectChildList* children = virtualChildren()) > + return children->before(this); > + return 0; > + } > + RenderObject* after() const > + { > + if (const RenderObjectChildList* children = virtualChildren()) > + return children->after(this); > + return 0; > + } These names are just too generic. They sound like you're getting the previous and next siblings. I think you need to rename them to beforePseudoElementContainer() and afterPseudoElementContainer(). > Source/WebCore/rendering/RenderObject.h:463 > + Node* generatingNode() const { return m_node == document() ? 0 : m_node; } Ultimately this should be set correctly for all pseudo-elements. I think it's ok to limit it to before/after as a first cut, but we should have a follow-up bug for correcting this for the other pseudo-elements.

Comment on attachment 79956 [details] Proposed Patch. Fixed chromium build issue Looking at this further, I don't really think you need to touch RenderObject.h. You're just slowing the code down by making RenderObjectChildList::beforeAfterContainer go back through the RenderObject to fetch virtualChildren(), which is the same list you're already dealing with.

Created attachment 80288 [details] Proposed Patch Refactored beforeAfterContainer (which was not a member of RenderObjectChildren, that is why it needed to go back to RenderObject to find the RenderObjectChildrenList). Renamed `before` and `after` methods to beforePseudoElementRenderer and afterPseudoElementRenderer respectively. Did not like `Container`, because the returned renderer does not contain the PseudoElement it is either the pseudoelement or its renderer depending on interpretation of what a pseudoelement is. I was always confused by the prior nomenclature thinking that the returned Renderer contains the before and the after pseudoelements' renderers.

Comment on attachment 80288 [details] Proposed Patch r=me

Comment on attachment 80288 [details] Proposed Patch Clearing flags on attachment: 80288 Committed r77191: <http://trac.webkit.org/changeset/77191>

All reviewed patches have been landed. Closing bug.