| Summary: | Add LayoutTest to ensure resources within <noscript> are not preloaded | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Tony Gentilcore <tonyg> | ||||||||
| Component: | New Bugs | Assignee: | Tony Gentilcore <tonyg> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | abarth, commit-queue, eric | ||||||||
| Priority: | P2 | ||||||||||
| Version: | 528+ (Nightly build) | ||||||||||
| Hardware: | Other | ||||||||||
| OS: | OS X 10.5 | ||||||||||
| Bug Depends on: | |||||||||||
| Bug Blocks: | 40779 | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Tony Gentilcore
2010-06-18 12:35:47 PDT
Created attachment 59144 [details]
Patch
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.
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. :( Yeah. Good idea. :) Created attachment 59153 [details]
Patch
Comment on attachment 59153 [details]
Patch
noscirpt-image2 or noscript-image3 are less likely to be re-used by some other test.
Created attachment 59156 [details]
Patch
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!
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 on attachment 59156 [details]
Patch
We'll find out. :)
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 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 (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. > 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.
Committed r61497: <http://trac.webkit.org/changeset/61497> |