Bug 49529 – [gtk] SVGLineElement-dom-requiredFeatures.html and SVGLineElement-svgdom-requiredFeatures.html failing in the bots

• Component: WebKitGTK
• Status: RESOLVED FIXED

[Alejandro G. Castro]

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?

[Rob Buis]

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.

[Alejandro G. Castro]

(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.

[Dirk Schulze]

(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..)

[Rob Buis]

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.

[Alejandro G. Castro]

(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.

[Alejandro G. Castro]

(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.

[Rob Buis]

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.

[Alejandro G. Castro]

Created attachment 74738 [details]
Proposed patch

I've created the patch on your behalf because I think we need a review for this one.

[WebKit Commit Bot]

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 [e-mail] and [e-mail].  The commit-queue is continuing to process your patch.

[WebKit Commit Bot]

Comment on attachment 74738 [details]
Proposed patch

Clearing flags on attachment: 74738

Committed r73005: <http://trac.webkit.org/changeset/73005>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.