Bug 40849 - Add LayoutTest to ensure resources within <noscript> are not preloaded
Summary: Add LayoutTest to ensure resources within <noscript> are not preloaded
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: Other OS X 10.5
: P2 Normal
Assignee: Tony Gentilcore
URL:
Keywords:
Depends on:
Blocks: 40779
  Show dependency treegraph
 
Reported: 2010-06-18 12:35 PDT by Tony Gentilcore
Modified: 2010-06-19 13:39 PDT (History)
3 users (show)

See Also:


Attachments
Patch (2.29 KB, patch)
2010-06-18 12:38 PDT, Tony Gentilcore
no flags Details | Formatted Diff | Diff
Patch (2.58 KB, patch)
2010-06-18 13:58 PDT, Tony Gentilcore
no flags Details | Formatted Diff | Diff
Patch (2.68 KB, patch)
2010-06-18 14:10 PDT, Tony Gentilcore
eric: review+
commit-queue: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tony Gentilcore 2010-06-18 12:35:47 PDT
Add LayoutTest to ensure resources within <noscript> are not preloaded
Comment 1 Tony Gentilcore 2010-06-18 12:38:57 PDT
Created attachment 59144 [details]
Patch
Comment 2 Adam Barth 2010-06-18 13:07:17 PDT
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.
Comment 3 Eric Seidel (no email) 2010-06-18 13:12:57 PDT
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. :(
Comment 4 Adam Barth 2010-06-18 13:19:05 PDT
Yeah.  Good idea.  :)
Comment 5 Tony Gentilcore 2010-06-18 13:58:16 PDT
Created attachment 59153 [details]
Patch
Comment 6 Eric Seidel (no email) 2010-06-18 14:01:46 PDT
Comment on attachment 59153 [details]
Patch

noscirpt-image2 or noscript-image3 are less likely to be re-used by some other test.
Comment 7 Tony Gentilcore 2010-06-18 14:10:20 PDT
Created attachment 59156 [details]
Patch
Comment 8 Eric Seidel (no email) 2010-06-18 14:12:41 PDT
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!
Comment 9 Tony Gentilcore 2010-06-18 14:16:39 PDT
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?
Comment 10 Adam Barth 2010-06-18 14:53:29 PDT
Comment on attachment 59156 [details]
Patch

We'll find out.  :)
Comment 11 Adam Barth 2010-06-18 14:54:09 PDT
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.  :)
Comment 12 WebKit Commit Bot 2010-06-19 10:00:42 PDT
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
Comment 13 Tony Gentilcore 2010-06-19 10:47:18 PDT
(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.
Comment 14 Adam Barth 2010-06-19 12:42:52 PDT
> 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.
Comment 15 Adam Barth 2010-06-19 13:39:42 PDT
Committed r61497: <http://trac.webkit.org/changeset/61497>