Bug 40849 – Add LayoutTest to ensure resources within <noscript> are not preloaded

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Tony Gentilcore]

Add LayoutTest to ensure resources within <noscript> are not preloaded

[Tony Gentilcore]

Created attachment 59144 [details]
Patch

[Adam Barth]

Comment on attachment 59144 [details]
Patch

This might need to be landed by hand because of the empty file.

LayoutTests/fast/preloader/noscript.html:13
 +      <img src=resources/image1.png>
You need to use unique file names to ensure that these URLs aren't in the cache.  (We only get the resource response printout when we actually hit the network.)

LayoutTests/fast/preloader/noscript.html:11
 +  <script src=resources/non-existant.js></script>
Also, you should add the document.write("<plaintext>") inline script here to ensure that we're actually seeing the preloader in action.  The way this test is written currently, we'll pass without a preloader or if the preloader skips both.

LayoutTests/fast/preloader/noscript.html:15
 +  <img src=resources/image2.png>
Also, feel free to use double-quoted attributes or whatever so we have some more coverage of the preload scanner's tokenizer.

[Eric Seidel (no email)]

Adam, you should consider adding a README to fast/preloader to explain how to write these tests.  Things like unique resource names are not immediately obvious. :(

[Adam Barth]

Yeah.  Good idea.  :)

[Tony Gentilcore]

Created attachment 59153 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 59153 [details]
Patch

noscirpt-image2 or noscript-image3 are less likely to be re-used by some other test.

[Tony Gentilcore]

Created attachment 59156 [details]
Patch

[Eric Seidel (no email)]

Comment on attachment 59156 [details]
Patch

LGTM.  I would have commented in the test that one should only see load messages for image2, not image1.  BUt this is also OK as-is.

Thanks!

[Tony Gentilcore]

Adam mentioned something about an empty file and landing by hand? Is this safe to cq+ or do one of you guys need to land it?

[Adam Barth]

Comment on attachment 59156 [details]
Patch

We'll find out.  :)

[Adam Barth]

Tony, in case you're writing more of these tests, you can only have it print out one successful load (like you did in this test) because the order of the print statements is non-deterministic.  :)

[WebKit Commit Bot]

Comment on attachment 59156 [details]
Patch

Rejecting patch 59156 from commit-queue.

Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 2
Last 500 characters of output:

patching file LayoutTests/ChangeLog
Hunk #1 succeeded at 1 with fuzz 3.
patching file LayoutTests/fast/preloader/noscript-expected.txt
patching file LayoutTests/fast/preloader/noscript.html
patch: **** Only garbage was found in the patch input.
fatal: pathspec 'LayoutTests/fast/preloader/resources/noscript-image1.png' did not match any files
Failed to git add LayoutTests/fast/preloader/resources/noscript-image1.png. at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply line 427.

Full output: http://webkit-commit-queue.appspot.com/results/3335400

[Tony Gentilcore]

(In reply to comment #12)
> (From update of attachment 59156 [details])
> Rejecting patch 59156 from commit-queue.
> 
> Failed to run "[u'/Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply', u'--reviewer', u'Eric Seidel', u'--force']" exit_code: 2
> Last 500 characters of output:
> 
> patching file LayoutTests/ChangeLog
> Hunk #1 succeeded at 1 with fuzz 3.
> patching file LayoutTests/fast/preloader/noscript-expected.txt
> patching file LayoutTests/fast/preloader/noscript.html
> patch: **** Only garbage was found in the patch input.
> fatal: pathspec 'LayoutTests/fast/preloader/resources/noscript-image1.png' did not match any files
> Failed to git add LayoutTests/fast/preloader/resources/noscript-image1.png. at /Users/eseidel/Projects/CommitQueue/WebKitTools/Scripts/svn-apply line 427.
> 
> Full output: http://webkit-commit-queue.appspot.com/results/3335400

Looks like that answered our question about whether the CQ can land images.

[Adam Barth]

> Looks like that answered our question about whether the CQ can land images.

It's not images, per se, it's empty files.  I'll land by hand.

[Adam Barth]

Committed r61497: <http://trac.webkit.org/changeset/61497>