RESOLVED FIXED 134788
[iOS] Some QuickLook documents are not displayed as previews
https://bugs.webkit.org/show_bug.cgi?id=134788
Summary [iOS] Some QuickLook documents are not displayed as previews
Andy Estes
Reported 2014-07-09 17:41:58 PDT
[iOS] Some QuickLook documents are not displayed as previews
Attachments
Patch (2.35 KB, patch)
2014-07-09 17:55 PDT, Andy Estes
psolanki: review+
Andy Estes
Comment 1 2014-07-09 17:53:48 PDT
Andy Estes
Comment 2 2014-07-09 17:55:18 PDT
Pratik Solanki
Comment 3 2014-07-09 23:21:38 PDT
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 4 2014-07-10 04:24:56 PDT
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
Comment 5 2014-07-10 08:36:00 PDT
(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
Comment 6 2014-07-10 08:38:22 PDT
(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
Comment 7 2014-07-10 17:03:19 PDT
Note You need to log in before you can comment on or make changes to this bug.