[webkit-reviews] review denied: [Bug 45523] [Qt] Inconsistent behavior on a form submit request... : [Attachment 95038] Patch

[bugzilla-daemon at webkit.org]

Simon Hausmann <hausmann at webkit.org> has denied Diego Gonzalez
<diegohcg at webkit.org>'s request for review:
Bug 45523: [Qt] Inconsistent behavior on a form submit request...
https://bugs.webkit.org/show_bug.cgi?id=45523

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

------- Additional Comments from Simon Hausmann <hausmann at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=95038&action=review

r- because of the string literal.

With this change all pages are automatically in the same page group, which to
some extend this bug is about. This is a behavioural change that we should
really verify with a test.

I see that the Gtk+ port seems to behave the same, but I don't see the same
behaviour in the other ports directly (as they have an API). What's the pattern
their callers use? What's the safest default for us?

> Source/WebKit/qt/Api/qwebpage.cpp:159
> +const char* QWebPagePrivate::pageGroupName = "QtWebKit";

As a side-note: This declares a variable that is initialized to a read-only
string. The variable itself is writable however, which means it will
unnecessarily end up in the data section of unshared memory.

> Source/WebKit/qt/Api/qwebpage.cpp:343
> +    page->setGroupName(QWebPagePrivate::pageGroupName);

I think we should use a string literal here, instead of a variable.