Bug 25629 - [Gtk] Enable database and localStorage support
Summary: [Gtk] Enable database and localStorage support
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKitGTK (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2009-05-07 13:28 PDT by Jan Alonzo
Modified: 2009-05-23 15:40 PDT (History)
2 users (show)

See Also:


Attachments
add settings for database and localStorage (10.10 KB, patch)
2009-05-07 13:29 PDT, Jan Alonzo
no flags Details | Formatted Diff | Diff
remove DATABASE conditional guards (11.20 KB, patch)
2009-05-07 16:26 PDT, Jan Alonzo
no flags Details | Formatted Diff | Diff
updated patch to rename to properties to enable-html5-* (11.51 KB, patch)
2009-05-12 06:28 PDT, Jan Alonzo
gustavo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jan Alonzo 2009-05-07 13:28:24 PDT
Add settings to enable database and localStorage support and enable it by default. This allows databases and storage to be shown in the WebInspector and at the same time allow us to unskip most of the storage/ layout tests.
Comment 1 Jan Alonzo 2009-05-07 13:29:46 PDT
Created attachment 30114 [details]
add settings for database and localStorage
Comment 2 Christian Dywan 2009-05-07 15:31:13 PDT
These properties shouldn't be conditional. Rather document that the feature may be unavailable.

As for the two options, could you elaborate on what exactly these affect? I have a rough idea of DOM storage but I'm not clear why this need two different settings.
Comment 3 Jan Alonzo 2009-05-07 16:26:28 PDT
Created attachment 30117 [details]
remove DATABASE conditional guards

This patch removes the ENABLE(DATABASE) guard. Also improved on the documentation as suggested in irc by xan
Comment 4 Jan Alonzo 2009-05-07 16:31:21 PDT
(In reply to comment #2)
> These properties shouldn't be conditional. Rather document that the feature may
> be unavailable.

Updated the patch to remove the conditional.

> 
> As for the two options, could you elaborate on what exactly these affect? I
> have a rough idea of DOM storage but I'm not clear why this need two different
> settings.

"enable-databases" is for the Database API which uses SQL and is asynchronous.
"enable-local-storage" is synchronous and for simple storage. Examples in the web for localStorage seems that it's for storing key-value pairs i.e, localStorage["presence"] = "online".

Comment 5 Gustavo Noronha (kov) 2009-05-11 07:12:40 PDT
Comment on attachment 30117 [details]
remove DATABASE conditional guards

> +    /**
> +    * WebKitWebSettings:enable-databases:
> +    *
> +    * Whether to enable HTML5 client-side SQL database support. Client-side
> +    * SQL database allows web pages to store structured data and be able to
> +    * use SQL to manipulate that data asynchronously. Disabling this setting
> +    * will prevent if from appearing in the Database profile of the
> +    * WebInspector.
> +    *
> +    * Since 1.1.7
> +    */

The indentation of the *'s seems to be off (I think this is a problem elsewhere in our code, so probably caused by copy/paste).

The patch looks good to me, so I'll say r=me, if Xan is not against. We have agreed to have at least two people look at the API bits, so I'm not marking it r+ as of yet.
Comment 6 Christian Dywan 2009-05-11 16:11:00 PDT
Come to think of it, we need something like databases-max-age as well. Otherwise this will be stored forever - which isn't good. Or alternatively a signal like "database-changed" in the way the SoupCookie works.
Jan, would you know how to arrange that?
Comment 7 Christian Dywan 2009-05-11 16:12:03 PDT
And thanks a lot for adding the documentation.
Comment 8 Jan Alonzo 2009-05-12 06:26:38 PDT
(In reply to comment #6)
> Come to think of it, we need something like databases-max-age as well.
> Otherwise this will be stored forever - which isn't good. Or alternatively a
> signal like "database-changed" in the way the SoupCookie works.
> Jan, would you know how to arrange that?

You raised a good point but I think storage duration should be handled in WebCore and should be spec'd as part of HTML5 database (if not already). Maybe something to ask webkit-dev as well?

Comment 9 Jan Alonzo 2009-05-12 06:28:34 PDT
Created attachment 30224 [details]
updated patch to rename to properties to enable-html5-*

Updated patch to add 'html5' to the property names to remove ambiguity with icon-database. Also remove mention of WebInspector in the doc as suggested by Xan in IRC.
Comment 10 Christian Dywan 2009-05-12 11:15:03 PDT
I sent a message to the WebKit devel list now, subject "Controling HTML5 local storage/ databases programmatically".
Comment 11 Xan Lopez 2009-05-23 06:32:44 PDT
Comment on attachment 30224 [details]
updated patch to rename to properties to enable-html5-*

This looks good to me now. Since it's new API I'll wait for kov to give it the final thumbs up.
Comment 12 Gustavo Noronha (kov) 2009-05-23 07:06:46 PDT
Comment on attachment 30224 [details]
updated patch to rename to properties to enable-html5-*

Yeah, looks good. Perhaps you could split the fix to case PROP_ENABLE_PRIVATE_BROWSING in a different commit, though? And remember to fix Since to be 1.1.8.
Comment 13 Jan Alonzo 2009-05-23 15:40:42 PDT
(In reply to comment #12)
> (From update of attachment 30224 [details] [review])
> Yeah, looks good. Perhaps you could split the fix to case
> PROP_ENABLE_PRIVATE_BROWSING in a different commit, though? And remember to fix
> Since to be 1.1.8.
> 

Thanks! Landed in r44105. Private browsing case fix landed in r44106.