Bug 167453 – [QuickLook] REGRESSION (WebKit2): Requests are made to invalid x-apple-ql-id: URLs

• Component: New Bugs
• Status: RESOLVED FIXED

[Andy Estes]

[QuickLook] REGRESSION (WebKit2): Requests are made to invalid x-apple-ql-id: URLs

[Andy Estes]

Created attachment 299801 [details]
Patch

[Brent Fulgham]

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?

[Andy Estes]

(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!

[Andy Estes]

Created attachment 299886 [details]
Patch

[WebKit Commit Bot]

Comment on attachment 299886 [details]
Patch

Clearing flags on attachment: 299886

Committed r211248: <http://trac.webkit.org/changeset/211248>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.