Bug 195011 – [WPE][GTK] Deprecate WebSQL APIs

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKitGTK

[Michael Catanzaro]

WebSQL is disabled at runtime since 2.24 and we don't have any way in our API to enable it. I don't think we want to add a way to do so, not unless some application actually needs it, which seems quite unlikely. So deprecate stuff.

Note: this is for 2.24

[Michael Catanzaro]

Created attachment 362916 [details]
Patch

[EWS Watchlist]

Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API

[EWS Watchlist]

Attachment 362916 [details] did not pass style-queue:


WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitWebsiteDataManager.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h"
WARNING: File exempt from style guide. Skipping: "Source/WebKit/UIProcess/API/gtk/WebKitWebsiteDataManager.h"
ERROR: Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:135:  Non-label code inside switch statements should be indented.  [whitespace/indent] [4]
Total errors found: 1 in 10 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Michael Catanzaro]

Ping for review.

[Adrian Perez]

According to https://caniuse.com/#feat=sql-storage the feature is
available still in Safari... I wonder if it might cause trouble due
to websites deciding to use WebSQL after doing UA sniffing and finding
that “looks like Safari” 🤔

On the other hand, Firefox does not have WebSQL, and I see that there
has been no work on the spec since 2010 so it seems like it should be
safe (ish?) to completely disable it.

Other than the commentary above, patch LGTM.

[Michael Catanzaro]

I think caniuse.com is outdated.

This patch doesn't disable WebSQL anyway. It only updates our API docs to reflect that WebSQL is already disabled since 2.24 and cannot be enabled. Currently our API docs are wrong.

[Michael Catanzaro]

Ping reviewers. This is broken in 2.24.

[Carlos Garcia Campos]

Comment on attachment 362916 [details]
Patch

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

> Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:323
> +     * Deprecated: 2.24. WebSQL is no longer supported. Use IndexedDB instead.

2.26 now

> Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h:43
> + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).

WebSQL databases. Deprecated 2.26

> Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h:43
> - * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases.
> + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).

Ditto.

> Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:156
> +ALLOW_DEPRECATED_DECLARATIONS_BEGIN
>      GUniquePtr<char> webSQLDirectory(g_build_filename(Test::dataDirectory(), "websql", nullptr));
>      g_assert_cmpstr(webSQLDirectory.get(), ==, webkit_website_data_manager_get_websql_directory(test->m_manager));
>      test->runJavaScriptAndWaitUntilFinished("db = openDatabase(\"TestDatabase\", \"1.0\", \"TestDatabase\", 1);", nullptr);
>      g_assert_true(g_file_test(webSQLDirectory.get(), G_FILE_TEST_IS_DIR));
> +ALLOW_DEPRECATED_DECLARATIONS_END

Let's remove the tests, since it does nothing in the end.

[Carlos Garcia Campos]

hmm, since it's not only deprecated but disabled, we might consider to add a g_warning() to let the user know that WebSQL is not going to work anyway. And document that as well. Just saying it's deprecated sounds like it still works.

[Michael Catanzaro]

(In reply to Carlos Garcia Campos from comment #8)
> Comment on attachment 362916 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=362916&action=review
> 
> > Source/WebKit/UIProcess/API/glib/WebKitWebsiteDataManager.cpp:323
> > +     * Deprecated: 2.24. WebSQL is no longer supported. Use IndexedDB instead.
> 
> 2.26 now

No, it's really for 2.24, late or not. Remember WebSQL is not available at all since 2.24.

> > Source/WebKit/UIProcess/API/gtk/WebKitWebsiteData.h:43
> > + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).
> 
> WebSQL databases. Deprecated 2.26
> 
> > Source/WebKit/UIProcess/API/wpe/WebKitWebsiteData.h:43
> > - * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases.
> > + * @WEBKIT_WEBSITE_DATA_WEBSQL_DATABASES: WebSQL databases (deprecated).
> 
> Ditto.

OK (2.24)

> > Tools/TestWebKitAPI/Tests/WebKitGLib/TestWebsiteData.cpp:156
> > +ALLOW_DEPRECATED_DECLARATIONS_BEGIN
> >      GUniquePtr<char> webSQLDirectory(g_build_filename(Test::dataDirectory(), "websql", nullptr));
> >      g_assert_cmpstr(webSQLDirectory.get(), ==, webkit_website_data_manager_get_websql_directory(test->m_manager));
> >      test->runJavaScriptAndWaitUntilFinished("db = openDatabase(\"TestDatabase\", \"1.0\", \"TestDatabase\", 1);", nullptr);
> >      g_assert_true(g_file_test(webSQLDirectory.get(), G_FILE_TEST_IS_DIR));
> > +ALLOW_DEPRECATED_DECLARATIONS_END
> 
> Let's remove the tests, since it does nothing in the end.

OK

[Michael Catanzaro]

Created attachment 371954 [details]
Patch

[Michael Catanzaro]

(cq? to see if you're OK with the 2.24 deprecations. I know it's odd to use these retroactively, but it's weird to say it's deprecated in 2.26 as that might trick people into thinking it's available in 2.24.)

[Michael Catanzaro]

(In reply to Michael Catanzaro from comment #12)
> (cq? to see if you're OK with the 2.24 deprecations. I know it's odd to use
> these retroactively, but it's weird to say it's deprecated in 2.26 as that
> might trick people into thinking it's available in 2.24.)

BTW I do intend this for 2.24 branch as well.

[WebKit Commit Bot]

Comment on attachment 371954 [details]
Patch

Clearing flags on attachment: 371954

Committed r246353: <https://trac.webkit.org/changeset/246353>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.