Bug 60721 - Marker test from ietestcenter fails
: Marker test from ietestcenter fails
Status: RESOLVED FIXED
: WebKit
SVG
: 528+ (Nightly build)
: Unspecified Unspecified
: P2 Normal
Assigned To:
: http://samples.msdn.microsoft.com/iet...
:
:
:
  Show dependency treegraph
 
Reported: 2011-05-12 13:25 PST by
Modified: 2011-05-19 07:46 PST (History)


Attachments
Patch (23.07 KB, patch)
2011-05-12 13:46 PST, Rob Buis
no flags Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-02 (1.24 MB, application/zip)
2011-05-12 14:10 PST, WebKit Review Bot
no flags Details
Patch (23.49 KB, patch)
2011-05-12 14:28 PST, Rob Buis
no flags Review Patch | Details | Formatted Diff | Diff
Patch (55.38 KB, patch)
2011-05-12 14:56 PST, Rob Buis
no flags Review Patch | Details | Formatted Diff | Diff
Patch (41.27 KB, patch)
2011-05-12 15:13 PST, Rob Buis
zimmermann: review+
webkit.review.bot: commit‑queue-
Review Patch | Details | Formatted Diff | Diff
Archive of layout-test-results from ec2-cr-linux-01 (1.24 MB, application/zip)
2011-05-12 15:54 PST, WebKit Review Bot
no flags Details


Note

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


Description From 2011-05-12 13:25:01 PST
See the test url, this test also is part of SVG1.12nd edition testsuite.
------- Comment #1 From 2011-05-12 13:46:47 PST -------
Created an attachment (id=93330) [details]
Patch
------- Comment #2 From 2011-05-12 13:50:33 PST -------
Your ChangeLog is missing an explanation of why this change is correct.
------- Comment #3 From 2011-05-12 13:57:50 PST -------
(From update of attachment 93330 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=93330&action=review

> Source/WebCore/svg/SVGMarkerElement.cpp:251
> +    return style->display() != NONE || (parentNode() && parentNode()->renderer() && parentNode()->renderer()->isSVGHiddenContainer());

What about visibility?
------- Comment #4 From 2011-05-12 14:10:10 PST -------
(From update of attachment 93330 [details])
Attachment 93330 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8684889

New failing tests:
svg/W3C-SVG-1.1-SE/painting-marker-07-f.svg
------- Comment #5 From 2011-05-12 14:10:16 PST -------
Created an attachment (id=93336) [details]
Archive of layout-test-results from ec2-cr-linux-02

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-02  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #6 From 2011-05-12 14:28:03 PST -------
Created an attachment (id=93344) [details]
Patch
------- Comment #7 From 2011-05-12 14:37:03 PST -------
(From update of attachment 93344 [details])
View in context: https://bugs.webkit.org/attachment.cgi?id=93344&action=review

I'd like to see at least one more test, where you use inherit for display <marker style="display:inherit" ...

r- because of the new test and the changelog.

> Source/WebCore/ChangeLog:9
> +
> +        Allow <marker> to be referenced in case the display:none is set on it
> +        or any of the ancestors.

You're doing more now. Marker gets a Renderer, always. Also not sure what you mean with referenced in the combination with display:none. Please add a more detailed description what you changed, why you did it, Why it fixes the problem. Add a link to the specification text.
------- Comment #8 From 2011-05-12 14:56:58 PST -------
Created an attachment (id=93349) [details]
Patch
------- Comment #9 From 2011-05-12 15:13:55 PST -------
Created an attachment (id=93353) [details]
Patch
------- Comment #10 From 2011-05-12 15:54:26 PST -------
(From update of attachment 93353 [details])
Attachment 93353 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/8689847

New failing tests:
svg/W3C-SVG-1.1-SE/types-dom-05-b.svg
svg/W3C-SVG-1.1-SE/painting-marker-07-f.svg
------- Comment #11 From 2011-05-12 15:54:31 PST -------
Created an attachment (id=93361) [details]
Archive of layout-test-results from ec2-cr-linux-01

The attached test failures were seen while running run-webkit-tests on the chromium-ews.
Bot: ec2-cr-linux-01  Port: Chromium  Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
------- Comment #12 From 2011-05-18 07:26:47 PST -------
(From update of attachment 93353 [details])
Looks great, r=me.
------- Comment #13 From 2011-05-18 14:40:36 PST -------
Committed r86791: <http://trac.webkit.org/changeset/86791>
------- Comment #14 From 2011-05-18 18:55:21 PST -------
It seems like we need a Windows rebaseline:
http://build.webkit.org/builders/Windows%20XP%20Debug%20%28Tests%29/builds/28783
------- Comment #16 From 2011-05-19 07:46:59 PST -------
Hi Ryosuke,

(In reply to comment #14)
> It seems like we need a Windows rebaseline:
> http://build.webkit.org/builders/Windows%20XP%20Debug%20%28Tests%29/builds/28783

Agreed! I want to make clear as well that the new result is an improvement.
Cheers,

Rob.