Bug 49529 - [gtk] SVGLineElement-dom-requiredFeatures.html and SVGLineElement-svgdom-requiredFeatures.html failing in the bots
Summary: [gtk] SVGLineElement-dom-requiredFeatures.html and SVGLineElement-svgdom-requ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-11-15 02:31 PST by Alejandro G. Castro
Modified: 2010-11-30 19:38 PST (History)
3 users (show)

See Also:


Attachments
Proposed patch (2.98 KB, patch)
2010-11-24 04:03 PST, Alejandro G. Castro
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alejandro G. Castro 2010-11-15 02:31:01 PST
They work for me locally but apparently the startTest does not start the test in the bots. I'm skipping the tests until we find the issue. Dirk do you have any clue about why this could happen?
Comment 1 Rob Buis 2010-11-15 13:00:30 PST
Hi,

(In reply to comment #0)
> They work for me locally but apparently the startTest does not start the test in the bots. I'm skipping the tests until we find the issue. Dirk do you have any clue about why this could happen?

This sounds like the onclick is not triggered. You could try such a change to see if it helps:

diff --git a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
index b1f8e95..ccbc735 100644
--- a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
+++ b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
@@ -9,6 +9,7 @@ lineElement.setAttribute("y1", "20");
 lineElement.setAttribute("x2", "200");
 lineElement.setAttribute("y2", "200");
 lineElement.setAttribute("fill", "green");
+lineElement.setAttribute("style", "stroke-width:10px");

 rootSVGElement.appendChild(lineElement);

diff --git a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
index 2350445..ffab290 100644
--- a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
+++ b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
@@ -9,6 +9,7 @@ lineElement.setAttribute("y1", "20");
 lineElement.setAttribute("x2", "200");
 lineElement.setAttribute("y2", "200");
 lineElement.setAttribute("fill", "green");
+lineElement.setAttribute("style", "stroke-width:10px");

 rootSVGElement.appendChild(lineElement);

Cheers,

Rob.
Comment 2 Alejandro G. Castro 2010-11-17 04:34:10 PST
(In reply to comment #1)
> Hi,
> 
> (In reply to comment #0)
> > They work for me locally but apparently the startTest does not start the test in the bots. I'm skipping the tests until we find the issue. Dirk do you have any clue about why this could happen?
> 
> This sounds like the onclick is not triggered. You could try such a change to see if it helps:
> 

Thanks for the help, I tried the change in the bots and apparently did not fix the issue, I also tried to change the point of the onclick but without success.
Comment 3 Dirk Schulze 2010-11-17 11:03:06 PST
(In reply to comment #2)
> (In reply to comment #1)
> > Hi,
> > 
> > (In reply to comment #0)
> > > They work for me locally but apparently the startTest does not start the test in the bots. I'm skipping the tests until we find the issue. Dirk do you have any clue about why this could happen?
> > 
> > This sounds like the onclick is not triggered. You could try such a change to see if it helps:
> > 
> 
> Thanks for the help, I tried the change in the bots and apparently did not fix the issue, I also tried to change the point of the onclick but without success.

Does it work manually for you? Means opening the test in the browser and click manually on the element? (I'm not sure if the element is visible at the beginning of the test, so..)
Comment 4 Rob Buis 2010-11-17 12:39:43 PST
Hi,

(In reply to comment #2)
> (In reply to comment #1)
> > Hi,
> > 
> > (In reply to comment #0)
> > > They work for me locally but apparently the startTest does not start the test in the bots. I'm skipping the tests until we find the issue. Dirk do you have any clue about why this could happen?
> > 
> > This sounds like the onclick is not triggered. You could try such a change to see if it helps:
> > 
> 
> Thanks for the help, I tried the change in the bots and apparently did not fix the issue, I also tried to change the point of the onclick but without success.

Dirks tip to try it manually is a good idea. Also I noticed setting fill is probably not correct for the line, could you try this patch instead:

diff --git a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
index b1f8e95..3a54c7a 100644
--- a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
+++ b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-dom-requiredFeatures.js
@@ -8,7 +8,8 @@ lineElement.setAttribute("x1", "20");
 lineElement.setAttribute("y1", "20");
 lineElement.setAttribute("x2", "200");
 lineElement.setAttribute("y2", "200");
-lineElement.setAttribute("fill", "green");
+lineElement.setAttribute("stroke", "green");
+lineElement.setAttribute("stroke-width", "10px");

 rootSVGElement.appendChild(lineElement);

diff --git a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
index 2350445..2c0b9d0 100644
--- a/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
+++ b/LayoutTests/svg/dynamic-updates/script-tests/SVGLineElement-svgdom-requiredFeatures.js
@@ -8,7 +8,8 @@ lineElement.setAttribute("x1", "20");
 lineElement.setAttribute("y1", "20");
 lineElement.setAttribute("x2", "200");
 lineElement.setAttribute("y2", "200");
-lineElement.setAttribute("fill", "green");
+lineElement.setAttribute("stroke", "green");
+lineElement.setAttribute("stroke-width", "10px");

 rootSVGElement.appendChild(lineElement);

Cheers,

Rob.
Comment 5 Alejandro G. Castro 2010-11-23 04:41:06 PST
(In reply to comment #3)
>
> [...]
> 
> Does it work manually for you? Means opening the test in the browser and click manually on the element? (I'm not sure if the element is visible at the beginning of the test, so..)

Sorry for the late answer, too  much things in my plate, you are right it even did not show the element so I could not click. With the patch sent by Rob it worked manually.
Comment 6 Alejandro G. Castro 2010-11-23 04:42:39 PST
(In reply to comment #4)
>
> [...]
> 
> Dirks tip to try it manually is a good idea. Also I noticed setting fill is probably not correct for the line, could you try this patch instead:
> 

Awesome, it worked, both manually and in the bots :). Thanks again for taking care and sorry for the late reply.
Comment 7 Rob Buis 2010-11-23 12:40:42 PST
Hi Alejandro,

(In reply to comment #6)
> (In reply to comment #4)
> >
> > [...]
> > 
> > Dirks tip to try it manually is a good idea. Also I noticed setting fill is probably not correct for the line, could you try this patch instead:
> > 
> 
> Awesome, it worked, both manually and in the bots :). Thanks again for taking care and sorry for the late reply.

No problem, I created the problem in the first place so I may as well clean it up :) So I guess you'll commit the fix soon?
Cheers,

Rob.
Comment 8 Alejandro G. Castro 2010-11-24 04:03:24 PST
Created attachment 74738 [details]
Proposed patch

I've created the patch on your behalf because I think we need a review for this one.
Comment 9 WebKit Commit Bot 2010-11-30 19:15:07 PST
The commit-queue encountered the following flaky tests while processing attachment 74738 [details]:

fast/history/history-subframe-with-name.html
fast/preloader/script.html

Please file bugs against the tests.  These tests were authored by abarth@webkit.org and mihaip@chromium.org.  The commit-queue is continuing to process your patch.
Comment 10 WebKit Commit Bot 2010-11-30 19:38:45 PST
Comment on attachment 74738 [details]
Proposed patch

Clearing flags on attachment: 74738

Committed r73005: <http://trac.webkit.org/changeset/73005>
Comment 11 WebKit Commit Bot 2010-11-30 19:38:50 PST
All reviewed patches have been landed.  Closing bug.