[webkit-reviews] review denied: [Bug 240327] [GTK][WPE] Do not return pointer to disposed timezone string : [Attachment 459201] Patch

[bugzilla-daemon at webkit.org]

Michael Catanzaro <mcatanzaro at gnome.org> has denied Yury Semikhatsky
<yurys at chromium.org>'s request for review:
Bug 240327: [GTK][WPE] Do not return pointer to disposed timezone string
https://bugs.webkit.org/show_bug.cgi?id=240327

Attachment 459201: Patch

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




--- Comment #3 from Michael Catanzaro <mcatanzaro at gnome.org> ---
Comment on attachment 459201
  --> https://bugs.webkit.org/attachment.cgi?id=459201
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:396
> +	   if (const auto* override = g_value_get_string(value);
isTimeZoneValid(override))

IMO it's more readable if you use two ifs rather than a semicolon.

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:-1883
> -/**
> - * webkit_web_context_set_time_zone_override:
> - * @context: a #WebKitWebContext
> - * @time_zone_override: value to set
> - *
> - * Set the #WebKitWebContext:time-zone-override property. Refer to the IANA
database for valid
> - * specifiers, https://en.wikipedia.org/wiki/List_of_tz_database_time_zones
> - *
> - * Since: 2.38
> - */
> -void webkit_web_context_set_time_zone_override(WebKitWebContext* context,
const gchar* timeZoneOverride)

Hm, why are you removing this?

Since it's declared in public headers, you need to remove it from the headers
too.

> Source/WebKit/UIProcess/API/glib/WebKitWebContext.cpp:-1888
> -    context->priv->timeZoneOverride = String::fromUTF8(timeZoneOverride);

If you don't want to remove the setter, I think you can just do:

context->priv->timeZoneOverride = timeZoneOverride

?