[iOS] Some QuickLook documents are not displayed as previews

• Component: New Bugs
• Status: RESOLVED FIXED

[Andy Estes]

[iOS] Some QuickLook documents are not displayed as previews

[Andy Estes]

<rdar://problem/17278194>

[Andy Estes]

Created attachment 234677 [details]
Patch

[Pratik Solanki]

Comment on attachment 234677 [details]
Patch

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

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

Mention the radar number here as well.

> Source/WebCore/ChangeLog:9
> +        NSURLRequest have no way to set this bit properly) and should probably be removed. Instead of using

That would be great! Do you think we can update all uses of isMainResourceRequest() with your check below? It would be good to get rid of this iOS specific code.

[David Kilzer (:ddkilzer)]

Comment on attachment 234677 [details]
Patch

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

>> Source/WebCore/ChangeLog:9
>> +        NSURLRequest have no way to set this bit properly) and should probably be removed. Instead of using
> 
> That would be great! Do you think we can update all uses of isMainResourceRequest() with your check below? It would be good to get rid of this iOS specific code.

I think this logic should be wrapped in a well-named (inline) method since it's not immediately obvious from reading the code that you're checking for the main resource being loaded.

[Andy Estes]

(In reply to comment #3)
> (From update of attachment 234677 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234677&action=review
> 
> > Source/WebCore/ChangeLog:4
> > +        https://bugs.webkit.org/show_bug.cgi?id=134788
> 
> Mention the radar number here as well.
> 
> > Source/WebCore/ChangeLog:9
> > +        NSURLRequest have no way to set this bit properly) and should probably be removed. Instead of using
> 
> That would be great! Do you think we can update all uses of isMainResourceRequest() with your check below? It would be good to get rid of this iOS specific code.

I considered doing this, but couldn't find an easy way to do so. The other uses of isMainResourceRequest() have access to a ResourceHandle instead of a ResourceLoader, and there's no easy way to get a ResourceLoader from a ResourceHandle (you can only get a ResourceHandleClient*, which isn't necessarily a ResourceLoader).

For now, what do you think about me renaming isMainResourceRequest() to deprecatedIsMainResourceRequest() and adding a comment stating why it can't be used reliably with the Network Process?

[Andy Estes]

(In reply to comment #4)
> (From update of attachment 234677 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=234677&action=review
> 
> >> Source/WebCore/ChangeLog:9
> >> +        NSURLRequest have no way to set this bit properly) and should probably be removed. Instead of using
> > 
> > That would be great! Do you think we can update all uses of isMainResourceRequest() with your check below? It would be good to get rid of this iOS specific code.
> 
> I think this logic should be wrapped in a well-named (inline) method since it's not immediately obvious from reading the code that you're checking for the main resource being loaded.

Will do!

[Andy Estes]

Committed r170984: <http://trac.webkit.org/changeset/170984>