Bug 139453

Summary:

REGRESSION (r173272): When open PDF from Safari in iBooks, title is replaced to “QuickLookPDF-s72DbgAU-1”

Product:

WebKit

Reporter:

Antti Koivisto <koivisto>

Component:

Page Loading

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, psolanki, yongjun_zhang

Priority:

Keywords:

InRadar

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
patch	psolanki: review+

Description Antti Koivisto 2014-12-09 11:19:06 PST

Bug in CFNetwork code path

Comment 1 Antti Koivisto 2014-12-09 11:29:43 PST

Created attachment 242947 [details]
patch

Comment 2 Antti Koivisto 2014-12-09 11:30:32 PST

rdar://problem/19052192

Comment 3 Pratik Solanki 2014-12-09 11:57:56 PST

Comment on attachment 242947 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242947&action=review

> Source/WebCore/ChangeLog:4
> +        https://bugs.webkit.org/show_bug.cgi?id=139453

Can you add the radar link here as well.

Comment 4 Antti Koivisto 2014-12-09 12:07:11 PST

https://trac.webkit.org/r177031

Comment 5 Darin Adler 2014-12-09 14:08:54 PST

Comment on attachment 242947 [details]
patch

View in context: https://bugs.webkit.org/attachment.cgi?id=242947&action=review

> Source/WebCore/platform/network/cf/ResourceResponseCFNet.cpp:133
> -    RetainPtr<CFStringRef> suggestedFilename = adoptCF(CFURLResponseCopySuggestedFilename(m_cfResponse.get()));
> +    RetainPtr<CFStringRef> suggestedFilename = adoptCF(CFURLResponseCopySuggestedFilename(cfURLResponse()));
>      return suggestedFilename.get();

I think that code like this reads better without a local RetainPtr variable:

    return adoptCF(CFURLResponseCopySuggestedFilename(cfURLResponse())).get();

In fact, I would write this:

    CFURLResponseRef underlyingResponse = cfURLResponse();
    if (!underlyingResponse)
        return String();
    return adoptCF(CFURLResponseCopySuggestedFilename(underlyingResponse)).get();

Comment 6 Antti Koivisto 2014-12-09 16:35:15 PST

>     CFURLResponseRef underlyingResponse = cfURLResponse();
>     if (!underlyingResponse)
>         return String();
>     return adoptCF(CFURLResponseCopySuggestedFilename(underlyingResponse)).get();

I wouldn't call it 'underlying' though as it doesn't describe the role of a synthesized CFURLResponse very well.