| Summary: | NRWT: doesn't support webarchives | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Chris Rogers <crogers> | ||||||
| Component: | New Bugs | Assignee: | Dirk Pranke <dpranke> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | abarth, ap, dpranke, eric, ojan, tony | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Other | ||||||||
| OS: | OS X 10.5 | ||||||||
| Bug Depends on: | |||||||||
| Bug Blocks: | 34984, 58625 | ||||||||
| Attachments: |
|
||||||||
|
Description
Chris Rogers
2011-04-11 18:00:27 PDT
Created attachment 89133 [details]
Patch
patch looks good to me. What is special about audio files that they need Content-Length? We don't write it for any other types. (In reply to comment #3) > What is special about audio files that they need Content-Length? We don't write it for any other types. We write the Content-Length for image files. While it's true that run-webkit-tests could probably scan over the bytes and look for the #EOF sentinel, this makes the parsing on the client side trivial, and it doesn't seem to hurt much. The same dump() function that is modified here saves application/pdf and application/x-webarchive without Content-Length - why is audio different? (In reply to comment #5) > The same dump() function that is modified here saves application/pdf and application/x-webarchive without Content-Length - why is audio different? It's a good question. the NRWT code processes text/plain a line at a time. I'm not actually sure if or how it handles PDF and webarchives, so I'll double check. Okay, the NRWT code was skipping all of the webarchive files -> not so good. I am implementing support for them now. It appears that all of the .webarchive files manage to end with a newline, causing the subsequent "#EOF" sentinel from DRT to be on a line by itself, so NRWT continues to work. .WAV files do not (currently) make that guarantee, and so NRWT's existing output-parsing will break. I need to continue working on the patch and doing more testing (to confirm w/ PDFs and files of other content types, for example). I also need to double check the logic against ORWT to see if ORWT is doing something more sophisticated to find where the sentinels are. Thanks for asking the question that revealed this deficiency. cancelling the review for now. Okay, it turns out that I can make NRWT handle base64-encoded wav output without needing a content-length header, and I think it doesn't break anything else (see bug 61819). I *suspect* that all of the pdf and webarchive files end up having a newline at the end so they didn't end up tripping up NRWT, although NRWT doesn't currently support webarchives and I haven't investigated PDFs yet. I am going to change the subject and use this bug to track adding support for the other output types to NRWT so I can continue to investigate this, but this'll take this bug off the blocking path for webaudio. In response to Alexey's question in comment #5 (why is webaudio different from webarchive or pdf) it looks like all of the webarchive files's expected output ends in a newline, and we don't actually generate any pdf output at this point. (although there appear to be tests that once did, they all generate rendertrees + images now). ORWT's code says that it expects "#EOF" to show up on a separate line, so in the absence of any other patches, we'd have to modify the base64-encoded output to also include a trailing newline. That seems like kind of a goofy change, so instead I have modified ORWT to match against any line that ends in a #EOF$ and if there is content preceding the #EOF, stick it into the returned result (see bug 57992 for the patch in question). I will still leave this open until NRWT supports webarchives correctly. Created attachment 97225 [details]
Patch
Comment on attachment 97225 [details]
Patch
That's it? Seems simple. If this works, great.
The PDF tests were added by me and immediately disabled. They were for printing. TUrns out that PDFs encode system information (including username!) so it's not possible to generate them in a system agnostic way... or at least didn't seem that way. (In reply to comment #12) > (From update of attachment 97225 [details]) > That's it? Seems simple. If this works, great. Yeah, as far as I can tell, .webarchives are just XML, so all we had to do way make the code aware of the other extension. I've edited the subject to remove reference to PDFs. Committed r88986: <http://trac.webkit.org/changeset/88986> |