Bug 53037 - layoutTestController.counterValueForElementById does not return the correct value
Summary: layoutTestController.counterValueForElementById does not return the correct v...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Carol Szabo
URL:
Keywords:
Depends on: 53024
Blocks: 52126
  Show dependency treegraph
 
Reported: 2011-01-24 12:39 PST by Carol Szabo
Modified: 2011-02-01 12:28 PST (History)
1 user (show)

See Also:


Attachments
Proposed Patch (4.96 KB, patch)
2011-01-31 11:00 PST, Carol Szabo
no flags Details | Formatted Diff | Diff
Proposed Patch (5.00 KB, patch)
2011-02-01 09:17 PST, Carol Szabo
darin: review+
darin: commit-queue-
Details | Formatted Diff | Diff
Proposed Patch. Addressed darin's concerns (4.96 KB, patch)
2011-02-01 10:52 PST, Carol Szabo
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carol Szabo 2011-01-24 12:39:32 PST
When the :before and/or :after pseudo-elements of an element are enclosed in an anonymous renderer, inside their parent renderer, they are not found by the layoutTestController.counterValueForElementById function.
Comment 1 Carol Szabo 2011-01-24 14:07:52 PST
Have patch ready, waiting for my patch to 53024 to land as it depends on that.
Comment 2 Carol Szabo 2011-01-31 11:00:20 PST
Created attachment 80660 [details]
Proposed Patch

This is my proposed patch. Waiting for my patch to 53024 to be landed before I submit this for review and commit (or else it won't build).
Comment 3 Carol Szabo 2011-02-01 09:17:10 PST
Created attachment 80767 [details]
Proposed Patch
Comment 4 Darin Adler 2011-02-01 09:39:21 PST
Comment on attachment 80767 [details]
Proposed Patch

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

Feel free to have another committer change commit-queue from - to + if you decide you don’t want to make my suggested minor tweaks.

> Source/WebCore/rendering/RenderTreeAsText.cpp:773
> +    // The counter renderers should be children of
> +    // :before or :after pseudo-elements.

No reason to break this comment into two lines.

> Source/WebCore/rendering/RenderTreeAsText.cpp:780
> +        RenderObject* pseudoElement = renderer->beforePseudoElementRenderer();
> +        if (pseudoElement)
> +            writeCounterValuesFromChildren(stream, pseudoElement, isFirstCounter);
> +        pseudoElement = renderer->afterPseudoElementRenderer();
> +        if (pseudoElement)
> +            writeCounterValuesFromChildren(stream, pseudoElement, isFirstCounter);

I suggest putting the local variable definitions into the if statements.
Comment 5 Carol Szabo 2011-02-01 10:52:18 PST
Created attachment 80786 [details]
Proposed Patch. Addressed darin's concerns
Comment 6 Dave Hyatt 2011-02-01 11:51:05 PST
Comment on attachment 80786 [details]
Proposed Patch. Addressed darin's concerns

r=me
Comment 7 WebKit Commit Bot 2011-02-01 12:28:20 PST
Comment on attachment 80786 [details]
Proposed Patch. Addressed darin's concerns

Clearing flags on attachment: 80786

Committed r77273: <http://trac.webkit.org/changeset/77273>
Comment 8 WebKit Commit Bot 2011-02-01 12:28:25 PST
All reviewed patches have been landed.  Closing bug.