[webkit-reviews] review granted: [Bug 57794] REGRESSION (r64712): Microsoft Outlook 2011: original message contents not included when replying to an email. : [Attachment 88560] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Andy Estes <aestes at apple.com>'s
request for review:
Bug 57794: REGRESSION (r64712): Microsoft Outlook 2011: original message
contents not included when replying to an email.
https://bugs.webkit.org/show_bug.cgi?id=57794

Attachment 88560: Patch
https://bugs.webkit.org/attachment.cgi?id=88560&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=88560&action=review

> Source/WebCore/loader/FrameLoader.cpp:-796
> +    m_frame->injectUserScripts(InjectAtDocumentEnd);
> +
>      if (m_stateMachine.creatingInitialEmptyDocument())
>	   return;
>  
> -    m_frame->injectUserScripts(InjectAtDocumentEnd);

No explanation of this change. This is why per-file/per-function comments in
ChangeLog are better. You could have said why you changed this.

> Source/WebCore/page/Frame.cpp:522
> +    if (loader()->stateMachine()->creatingInitialEmptyDocument()
> +	   && !settings()->injectUserScriptsInInitialEmptyDocument())

I suggest combining into one line.

> Source/WebCore/page/Settings.cpp:175
> +    , m_injectUserScriptsInInitialEmptyDocument(false)

SInce this is a verb phrase, I think you should add the word “should” to its
name.

> Source/WebCore/page/Settings.h:390
> +	   void setInjectUserScriptsInInitialEmptyDocument(bool flag) {
m_injectUserScriptsInInitialEmptyDocument = flag; }
> +	   bool injectUserScriptsInInitialEmptyDocument() { return
m_injectUserScriptsInInitialEmptyDocument; }

Ditto.

> Source/WebCore/page/Settings.h:493
> +	   bool m_injectUserScriptsInInitialEmptyDocument : 1;

Ditto.