Bug 133904 - convertToUTF8String converts null string to empty string
Summary: convertToUTF8String converts null string to empty string
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-06-14 09:38 PDT by Hamish Mackenzie
Modified: 2014-12-09 09:10 PST (History)
4 users (show)

See Also:


Attachments
Patch (1.26 KB, patch)
2014-12-09 02:24 PST, Alberto Garcia
no flags Details | Formatted Diff | Diff
Patch (2.80 KB, patch)
2014-12-09 09:02 PST, Alberto Garcia
cgarcia: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Hamish Mackenzie 2014-06-14 09:38:06 PDT
This means it is not possible to use functions like webkit_dom_css_style_declaration_get_property_value to tell if a property has no value (or is just set to the empty string).

I think it could be fixed with...

 gchar* convertToUTF8String(WTF::String const& s)
 {
+    if (!s) return 0;
     return g_strdup(s.utf8().data());
 }
Comment 1 Carlos Garcia Campos 2014-08-12 23:48:48 PDT
I agree, and it's consistent with the rest of the API that uses NULL instead of "". My only concern is that it might break existing applications relying on DOM functions returning always valid pointers. We have just broken the API/ABI so maybe it's a good momento for a change like this one, though.
Comment 2 Alberto Garcia 2014-12-08 10:58:52 PST
I guess if we really want to do this and it's not too late already, then it's now or never.
Comment 3 Carlos Garcia Campos 2014-12-09 00:12:28 PST
I had forgotten this, we should have changed it before the 2.6 release, but people are still migrating, s we can probably change it now.
Comment 4 Alberto Garcia 2014-12-09 02:24:45 PST
Created attachment 242899 [details]
Patch
Comment 5 Alberto Garcia 2014-12-09 09:02:54 PST
Created attachment 242928 [details]
Patch

I added a test
Comment 6 Carlos Garcia Campos 2014-12-09 09:06:53 PST
Comment on attachment 242928 [details]
Patch

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

Thanks!

> Source/WebCore/bindings/gobject/ConvertToUTF8String.cpp:33
> +        return 0;

nullptr

> Source/WebCore/bindings/gobject/ConvertToUTF8String.cpp:35
>      return g_strdup(s.utf8().data());

We could probably do something like return s.isNull() ? nullptr : g_strdup(s.utf8().data());
Comment 7 Carlos Garcia Campos 2014-12-09 09:07:19 PST
We should merge this in the stable branch too.
Comment 8 Alberto Garcia 2014-12-09 09:10:48 PST
Committed r177019: <http://trac.webkit.org/changeset/177019>