Bug 226738 – [iOS] Sync Accessibility preferences

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Per Arne Vollan]

Sync Accessibility preferences in the WebContent process with the preferences in the UI process.

[Per Arne Vollan]

<rdar://77922839>

[Per Arne Vollan]

Created attachment 430776 [details]
Patch

[chris fleizach]

Comment on attachment 430776 [details]
Patch

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

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:408
> +    auto appId = WebCore::applicationBundleIdentifier().createCFString();

does text size adjustments already work? that gets stored in com.apple.UIKit

> Source/WebKit/WebProcess/WebProcess.h:568
> +    void accessibilityPreferencesDidChange(bool reduceMotionEnabled, bool increaseButtonLegibility, bool enhanceTextLegibility, bool darkenSystemColors, bool invertColorsEnabled);

should we make this a struct where each item is named and then the struct gets passed around instead of separate params?

[Per Arne Vollan]

Created attachment 430873 [details]
Patch

[Per Arne Vollan]

(In reply to chris fleizach from comment #3)
> Comment on attachment 430776 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430776&action=review
> 
> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:408
> > +    auto appId = WebCore::applicationBundleIdentifier().createCFString();
> 
> does text size adjustments already work? that gets stored in com.apple.UIKit
> 

Ah, should that be removed from the patch, then?

> > Source/WebKit/WebProcess/WebProcess.h:568
> > +    void accessibilityPreferencesDidChange(bool reduceMotionEnabled, bool increaseButtonLegibility, bool enhanceTextLegibility, bool darkenSystemColors, bool invertColorsEnabled);
> 
> should we make this a struct where each item is named and then the struct
> gets passed around instead of separate params?

Done!

Thanks for reviewing!

[Brent Fulgham]

Comment on attachment 430873 [details]
Patch

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

This looks like a good change. Should we use this as the basis to handle Caption preferences, too?

> Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:586
> +    auto* pool = reinterpret_cast<WebProcessPool*>(observer);

Can observer ever be null? Perhaps we should ASSERT it? Perhaps we should test for null before calling through it?

[Per Arne Vollan]

Created attachment 431074 [details]
Patch

[Per Arne Vollan]

Created attachment 431075 [details]
Patch

[Per Arne Vollan]

(In reply to Brent Fulgham from comment #6)
> Comment on attachment 430873 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=430873&action=review
> 
> This looks like a good change. Should we use this as the basis to handle
> Caption preferences, too?
> 

That's a good point! It the patch uploaded for landing, I have worked towards that goal.

> > Source/WebKit/UIProcess/Cocoa/WebProcessPoolCocoa.mm:586
> > +    auto* pool = reinterpret_cast<WebProcessPool*>(observer);
> 
> Can observer ever be null? Perhaps we should ASSERT it? Perhaps we should
> test for null before calling through it?

I don't think observer can be null, since we subscribe to the notification with 'this'.

Thanks for reviewing!

[Per Arne Vollan]

Created attachment 431077 [details]
Patch

[Per Arne Vollan]

Created attachment 431079 [details]
Patch

[EWS]

Committed r278745 (238706@main): <https://commits.webkit.org/238706@main>

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

[Per Arne Vollan]

Reopening to attach new patch.

[Per Arne Vollan]

Created attachment 431195 [details]
Patch

[EWS]

Committed r278824 (238777@main): <https://commits.webkit.org/238777@main>

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