Bug 133072

Summary: http/tests/navigation/forward-to-fragment-fires-onload.html asserts
Product: WebKit Reporter: Jonathan Wells <jonowells>
Component: Tools / TestsAssignee: Anders Carlsson <andersca>
Status: REOPENED ---    
Severity: Normal CC: aestes, andersca, ap, commit-queue
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Attachments:
Description Flags
Patch
none
Patch none

Description Jonathan Wells 2014-05-19 09:51:35 PDT
Paths to js-test-pre.js and js-test-post.js appear to be incorrect.
Comment 1 Jonathan Wells 2014-05-19 10:01:21 PDT
Created attachment 231697 [details]
Patch
Comment 2 Andy Estes 2014-05-19 10:10:37 PDT
Comment on attachment 231697 [details]
Patch

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

> LayoutTests/http/tests/navigation/forward-to-fragment-fires-onload.html:11
> -description('Tests that loading is not stopped by going forward to a fragment.');
> +description("Tests to see that loading is not stopped by going forward to a fragment.");

I wouldn't change these to double quotes. It'll just make it harder for someone to blame this file later on.

> LayoutTests/http/tests/navigation/forward-to-fragment-fires-onload.html:17
> -    if (window.localStorage.stage == 'three') {
> -        console.log('3. Got back to start.  Going forward to page 2.');
> -        window.localStorage.stage = 'four';
> +    if (window.localStorage.stage == "three") {
> +        console.log("3. Got back to start.  Going forward to page 2.");
> +        window.localStorage.stage = "four";

Ditto.

> LayoutTests/http/tests/navigation/forward-to-fragment-fires-onload.html:19
> -    } else if (window.localStorage.stage == 'six') {
> +    } else if (window.localStorage.stage == "six") {

Ditto.

> LayoutTests/http/tests/navigation/forward-to-fragment-fires-onload.html:27
> -        console.log('Starting test.');
> -        window.localStorage.stage = 'one';
> +        console.log("Starting test.");
> +        window.localStorage.stage = "one";

Ditto.

> LayoutTests/http/tests/navigation/forward-to-fragment-fires-onload.html:30
> -            window.location.href = 'resources/forward-to-fragment-fires-onload-2.html';
> +            window.location.href = "resources/forward-to-fragment-fires-onload-2.html";

Ditto.
Comment 3 Jonathan Wells 2014-05-19 10:12:24 PDT
Created attachment 231698 [details]
Patch
Comment 4 Alexey Proskuryakov 2014-05-19 10:49:35 PDT
This test crashes on bots - is there more to the bug than the paths?

http://build.webkit.org/results/Apple%20MountainLion%20Debug%20WK2%20(Tests)/r169042%20(17740)/http/tests/navigation/forward-to-fragment-fires-onload-crash-log.txt
Comment 5 WebKit Commit Bot 2014-05-19 10:51:20 PDT
Comment on attachment 231698 [details]
Patch

Clearing flags on attachment: 231698

Committed r169050: <http://trac.webkit.org/changeset/169050>
Comment 6 WebKit Commit Bot 2014-05-19 10:51:26 PDT
All reviewed patches have been landed.  Closing bug.
Comment 7 Jonathan Wells 2014-05-19 11:27:58 PDT
I ran the tests and didn't have a crash or failure on this test anymore. However http/tests/navigation/resources/forward-to-fragment-fires-onload-2.html has a closing </iframe> tag that seems out of place.
Comment 8 Jonathan Wells 2014-05-19 12:20:14 PDT
Reopening as crash still occurring.
Comment 9 Jonathan Wells 2014-05-19 12:48:56 PDT
I think this was caused by http://trac.webkit.org/changeset/169026.
Comment 10 Alexey Proskuryakov 2014-05-28 15:48:50 PDT
The assertion was removed in <http://trac.webkit.org/changeset/169292>. As far as I can tell, that was done temporarily, and the assertion needs to be re-added.