[Webkit-unassigned] [Bug 166969] [SOUP] Fix handling of accept language property

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Jan 12 12:21:18 PST 2017


--- Comment #2 from Michael Catanzaro <mcatanzaro at igalia.com> ---
Comment on attachment 298672
  --> https://bugs.webkit.org/attachment.cgi?id=298672

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

I noticed this back before we supported network session. (That said, is it actually possible to have multiple network sessions in our port yet, or is that a future step?)

> Source/WebKit2/NetworkProcess/soup/NetworkProcessSoup.cpp:101
> +    g_object_set(NetworkStorageSession::defaultStorageSession().soupNetworkSession().soupSession(), "accept-language", acceptLanguages.data(), nullptr);
> +    NetworkStorageSession::forEach([acceptLanguages](const WebCore::NetworkStorageSession& session) {
> +        g_object_set(session.soupNetworkSession().soupSession(), "accept-language", acceptLanguages.data(), nullptr);


I think our SoupNetworkSession class should have a helper function that wraps this g_object_set(..., "accept-language", ...) so that you don't have to do this manually from outside the class. Up until this patch, that was SoupNetworkSession::setAcceptLanguages. So I really don't see the advantage to this approach. Instead of changing setAcceptLanguages to be static, I would call it here, and add a new static function SoupNetworkSession::setInitialAcceptLanguages to set it for new SoupNetworkSession objects. (I guess it's not possible to pass in via the constructor for some reason?)

This is just a matter of style; I'm willing to give r+ if you disagree, but I don't think this is the nicest way.

You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.webkit.org/pipermail/webkit-unassigned/attachments/20170112/620ac93e/attachment.html>

More information about the webkit-unassigned mailing list