Bug 33865 - svg/custom/stroke-width-click.svg fixes
Summary: svg/custom/stroke-width-click.svg fixes
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P2 Normal
Assignee: Nobody
Depends on:
Reported: 2010-01-19 13:53 PST by Garret Kelly
Modified: 2010-01-22 22:35 PST (History)
4 users (show)

See Also:

Initial patch (4.07 KB, patch)
2010-01-19 13:53 PST, Garret Kelly
no flags Details | Formatted Diff | Diff
Updated patch (4.85 KB, patch)
2010-01-21 12:23 PST, Garret Kelly
mjs: review-
Details | Formatted Diff | Diff
Updated patch (activate the test in platform-snowleopard) (5.49 KB, patch)
2010-01-22 06:28 PST, Garret Kelly
darin: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff
Updated patch (re-adds missing newline) (5.43 KB, patch)
2010-01-22 13:19 PST, Garret Kelly
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Garret Kelly 2010-01-19 13:53:25 PST
Created attachment 46950 [details]
Initial patch

Two issues:
- The way that stroke-width-click is written causes double-click events within the Chromium test_shell and triggers an ASSERT that is unrelated to the test.
- The output formatting does not allow for variances between platforms.

I've attached a patch that resolves this issue by reordering the click events so as to not trigger the double-click logic, and which changes the output formatting to not have rigid expectations.
Comment 1 Eric Seidel (no email) 2010-01-19 14:16:38 PST
You would need to include updating resutls if you were changing the test.

By "not judging" the results, its no longer clear from the test if it passed or failed.  I think that's bad.  If we have intentional platform-specific results those should be split out into another test instead of losing the value of having SUCCESS/FAIL for most of the test which is not platform-specific.
Comment 2 Garret Kelly 2010-01-21 12:23:31 PST
Created attachment 47137 [details]
Updated patch

I've investigated this bug a bit more, and it appears that the test has been disabled and flagged as a regression since Leopard. However, the output for Leopard (which agrees with Chromium and GTK (with the minor exception of another bug in the output, or else I'd re-enable it there too)) appears to be completely valid.

This patch updates the test to not trigger the double-click logic in the Chromium test_shell, disables the test under mac-tiger, enables the test under mac-leopard (it can probably be re-enabled on mac-snowleopard but I don't currently have access to that platform), fixes the expectation within the test about the the presence of the shape at (300, 150) and updates the expected file.
Comment 3 David Levin 2010-01-21 13:47:38 PST
Removed [chromium] since this is a generic change (even though the change may be done for chromium).
Comment 4 Maciej Stachowiak 2010-01-22 02:07:00 PST
Comment on attachment 47137 [details]
Updated patch

Looks good but I suggest enabling it on SnowLeopard too even if you have to do it blind. The odds that it will fail are pretty small. r- to consider that.
Comment 5 Garret Kelly 2010-01-22 06:28:18 PST
Created attachment 47198 [details]
Updated patch (activate the test in platform-snowleopard)

Thanks for the input Maciej! Here's an updated patch that removes the test from mac-snowleopard/Skipped but is otherwise identical to the previous one.
Comment 6 WebKit Commit Bot 2010-01-22 11:32:39 PST
Comment on attachment 47198 [details]
Updated patch (activate the test in platform-snowleopard)

Rejecting patch 47198 from commit-queue.

Failed to run "['WebKitTools/Scripts/run-webkit-tests', '--no-launch-safari', '--exit-after-n-failures=1', '--quiet']" exit_code: 1
Running build-dumprendertree
Running tests from /Users/eseidel/Projects/CommitQueue/LayoutTests
Testing 12031 test cases.
svg/custom/stroke-width-click.svg -> failed

Exiting early after 1 failures. 9696 tests run.
208.00s total testing time

9695 test cases (99%) succeeded
1 test case (<1%) had incorrect layout
6 test cases (<1%) had stderr output

Full output: http://webkit-commit-queue.appspot.com/results/204917
Comment 7 Garret Kelly 2010-01-22 13:19:20 PST
Created attachment 47221 [details]
Updated patch (re-adds missing newline)

This patch is identical to the last, except it has the missing newline that I accidentally stripped out.
Comment 8 WebKit Commit Bot 2010-01-22 22:35:27 PST
Comment on attachment 47221 [details]
Updated patch (re-adds missing newline)

Clearing flags on attachment: 47221

Committed r53762: <http://trac.webkit.org/changeset/53762>
Comment 9 WebKit Commit Bot 2010-01-22 22:35:34 PST
All reviewed patches have been landed.  Closing bug.