[webkit-reviews] review granted: [Bug 202012] Regression(r248832): Unable to quicklook HTML files in Mail : [Attachment 379173] Patch

[bugzilla-daemon at webkit.org]

Brent Fulgham <bfulgham at webkit.org> has granted Chris Dumez
<cdumez at apple.com>'s request for review:
Bug 202012: Regression(r248832): Unable to quicklook HTML files in Mail
https://bugs.webkit.org/show_bug.cgi?id=202012

Attachment 379173: Patch

https://bugs.webkit.org/attachment.cgi?id=379173&action=review




--- Comment #5 from Brent Fulgham <bfulgham at webkit.org> ---
Comment on attachment 379173
  --> https://bugs.webkit.org/attachment.cgi?id=379173
Patch

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

Thank you for jumping on this bug. r=me

> Source/WebKit/UIProcess/AuxiliaryProcessProxy.cpp:-225
> -	       }

Is this the Mail case?

> Source/WebKit/UIProcess/WebPageProxy.cpp:1186
> +    if (!process->isLaunching() || !url.isLocalFile())

Why don't we call into the WebPageProxy::loadFile code path when the url is a
local file? Are there cases that are "isLocalFile" but shouldn't use the file
loading path?

> Source/WebKit/UIProcess/WebPageProxy.cpp:1241
> +    if (!m_process->isLaunching())

This would be clearer if we flipped the check:

if (m_process->isLaunching())
    ... Send the 'LoadRequestWaitingForPID' message
else
    .... Send the 'LoadRequest' message.

But I guess that is the reverse of the "load from network' case.