Bug 226738 - [iOS] Sync Accessibility preferences
Summary: [iOS] Sync Accessibility preferences
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Per Arne Vollan
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-06-07 13:33 PDT by Per Arne Vollan
Modified: 2021-06-14 02:29 PDT (History)
7 users (show)

See Also:


Attachments
Patch (16.06 KB, patch)
2021-06-07 13:54 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (25.50 KB, patch)
2021-06-08 12:21 PDT, Per Arne Vollan
bfulgham: review+
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (23.53 KB, patch)
2021-06-10 06:47 PDT, Per Arne Vollan
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (21.65 KB, patch)
2021-06-10 07:18 PDT, Per Arne Vollan
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (21.58 KB, patch)
2021-06-10 07:35 PDT, Per Arne Vollan
ews-feeder: commit-queue-
Details | Formatted Diff | Diff
Patch (21.58 KB, patch)
2021-06-10 07:53 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff
Patch (4.13 KB, patch)
2021-06-11 08:04 PDT, Per Arne Vollan
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Per Arne Vollan 2021-06-07 13:33:14 PDT
Sync Accessibility preferences in the WebContent process with the preferences in the UI process.
Comment 1 Per Arne Vollan 2021-06-07 13:33:32 PDT
<rdar://77922839>
Comment 2 Per Arne Vollan 2021-06-07 13:54:46 PDT
Created attachment 430776 [details]
Patch
Comment 3 chris fleizach 2021-06-07 14:00:30 PDT
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?
Comment 4 Per Arne Vollan 2021-06-08 12:21:57 PDT
Created attachment 430873 [details]
Patch
Comment 5 Per Arne Vollan 2021-06-08 12:28:49 PDT
(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!
Comment 6 Brent Fulgham 2021-06-08 14:09:54 PDT
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?
Comment 7 Per Arne Vollan 2021-06-10 06:47:08 PDT
Created attachment 431074 [details]
Patch
Comment 8 Per Arne Vollan 2021-06-10 07:18:59 PDT
Created attachment 431075 [details]
Patch
Comment 9 Per Arne Vollan 2021-06-10 07:25:06 PDT
(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!
Comment 10 Per Arne Vollan 2021-06-10 07:35:07 PDT
Created attachment 431077 [details]
Patch
Comment 11 Per Arne Vollan 2021-06-10 07:53:21 PDT
Created attachment 431079 [details]
Patch
Comment 12 EWS 2021-06-10 17:49:48 PDT
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].
Comment 13 Per Arne Vollan 2021-06-11 08:04:08 PDT
Reopening to attach new patch.
Comment 14 Per Arne Vollan 2021-06-11 08:04:10 PDT
Created attachment 431195 [details]
Patch
Comment 15 EWS 2021-06-14 02:29:04 PDT
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].