Summary: | [QuickLook] REGRESSION (WebKit2): Requests are made to invalid x-apple-ql-id: URLs | ||||||||
---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Andy Estes <aestes> | ||||||
Component: | New Bugs | Assignee: | Andy Estes <aestes> | ||||||
Status: | RESOLVED FIXED | ||||||||
Severity: | Normal | CC: | achristensen, bfulgham, cdumez, commit-queue, darin, dbates, ddkilzer, japhet, kling, sam | ||||||
Priority: | P2 | ||||||||
Version: | WebKit Nightly Build | ||||||||
Hardware: | Unspecified | ||||||||
OS: | Unspecified | ||||||||
See Also: |
https://bugs.webkit.org/show_bug.cgi?id=163240 https://bugs.webkit.org/show_bug.cgi?id=135651 |
||||||||
Attachments: |
|
Description
Andy Estes
2017-01-26 02:16:28 PST
Created attachment 299801 [details]
Patch
Comment on attachment 299801 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=299801&action=review Looks good, but I had one question about the code change in the connection implementation. r=me. > Source/WebCore/loader/ios/QuickLook.mm:-341 > - [self _sendDidReceiveResponseIfNecessary]; I think I understand everything about this patch, except for this. Do we no longer emit necessary notifications without this send? (In reply to comment #2) > Comment on attachment 299801 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=299801&action=review > > Looks good, but I had one question about the code change in the connection > implementation. r=me. > > > Source/WebCore/loader/ios/QuickLook.mm:-341 > > - [self _sendDidReceiveResponseIfNecessary]; > > I think I understand everything about this patch, except for this. Do we no > longer emit necessary notifications without this send? I was trying to simplify things by directly calling ResourceLoader::didFail() directly without first calling didReceiveResponse(). In theory this shouldn't change behavior (in practice we can't even test this code path since QuickLook is configured to generate HTML error pages for non-password conversion failures). Doing this isn't necessary for this patch, though, so you're right to point it out. I'll add this call back. Thanks for the review! Created attachment 299886 [details]
Patch
Comment on attachment 299886 [details] Patch Clearing flags on attachment: 299886 Committed r211248: <http://trac.webkit.org/changeset/211248> All reviewed patches have been landed. Closing bug. |