Bug 211896 – Regression(r254856) Family Health iOS app is broken due to lack for WebSQL support

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Chris Dumez]

Family Health iOS app is broken due to lack for WebSQL support.

[Chris Dumez]

<rdar://problem/63025045>

[Chris Dumez]

Created attachment 399361 [details]
Patch

[Chris Dumez]

Comment on attachment 399361 [details]
Patch

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

> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;

Note that personally, I think we should use:
bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;

without any application checks. Please let me know if you agree / disagree.

[Maciej Stachowiak]

Comment on attachment 399361 [details]
Patch

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

>> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
>> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> 
> Note that personally, I think we should use:
> bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> 
> without any application checks. Please let me know if you agree / disagree.

Lack of application check creates a risk we'll have to keep rolling the OS version forward into the future, which makes it harder to ever delete WebSQL. So I think limiting it to specific apps is probably better. Though perhaps if there's enough apps, we could factor it out into a function so we don't end up with an unwieldy or expression.

[Maciej Stachowiak]

Comment on attachment 399361 [details]
Patch

Do we have a way to test this type of change, via API tests for instance?

Please add a test if possible.

Otherwise, r=me

[Chris Dumez]

(In reply to Maciej Stachowiak from comment #5)
> Comment on attachment 399361 [details]
> Patch
> 
> Do we have a way to test this type of change, via API tests for instance?
> 
> Please add a test if possible.
> 
> Otherwise, r=me

As far as I know, we still have test coverage for WebSQL being enabled.

[Chris Dumez]

(In reply to Maciej Stachowiak from comment #4)
> Comment on attachment 399361 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=399361&action=review
> 
> >> Source/WebKitLegacy/mac/WebView/WebPreferences.mm:399
> >> +    bool webSQLEnabled = (IOSApplication::isFamilyHealthApp() || IOSApplication::isJesusCalling()) && applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> > 
> > Note that personally, I think we should use:
> > bool webSQLEnabled = applicationSDKVersion() < DYLD_IOS_VERSION_14_0;
> > 
> > without any application checks. Please let me know if you agree / disagree.
> 
> Lack of application check creates a risk we'll have to keep rolling the OS
> version forward into the future, which makes it harder to ever delete
> WebSQL. So I think limiting it to specific apps is probably better. Though
> perhaps if there's enough apps, we could factor it out into a function so we
> don't end up with an unwieldy or expression.

Why would we keep rolling the IOS version forward? We drop support for WebSQL in a specific SDK, and then apps have to update if they want to build with the new SDK.

Checking only for specific apps is risky because we may not notice quickly all the apps that are broken without WebSQL or the breakage may be more subtle for certain apps. Also, it is quite a waste of engineering time to investigate app breakage and bisect it to this particular WebSQL change. I wasted a day on this and don't wish this on anybody else.

[EWS]

Committed r261695: <https://trac.webkit.org/changeset/261695>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 399361 [details].