Bug 225909

Summary: AX: Settings: Increase contrast isn't detected in browser until an additional setting is applied
Product: WebKit Reporter: chris fleizach <cfleizach>
Component: AccessibilityAssignee: chris fleizach <cfleizach>
Status: RESOLVED FIXED    
Severity: Normal CC: aboxhall, apinheiro, commit-queue, dmazzoni, eric.carlson, ews-watchlist, glenn, jcraig, jdiggs, jer.noble, philipj, pvollan, samuel_white, sergio, thorton, tsavell, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: All   
OS: All   
Bug Depends on: 226202    
Bug Blocks:    
Attachments:
Description Flags
patch
ews-feeder: commit-queue-
patch
ews-feeder: commit-queue-
patch
ews-feeder: commit-queue-
patch
ews-feeder: commit-queue-
patch
pvollan: review+
patch
none
patch
none
patch none

Description chris fleizach 2021-05-18 00:54:18 PDT 
When using a media query to detect the setting "Increase Contrast" AX setting, the setting can't be detected until an additional setting is updated.

<rdar://problem/77580880>
Comment 1 chris fleizach 2021-05-20 00:37:24 PDT 
Created attachment 429143 [details]
patch
Comment 2 chris fleizach 2021-05-20 00:55:24 PDT 
Created attachment 429144 [details]
patch
Comment 3 chris fleizach 2021-05-20 10:52:14 PDT 
Created attachment 429187 [details]
patch
Comment 4 chris fleizach 2021-05-20 11:00:15 PDT 
Created attachment 429188 [details]
patch
Comment 5 chris fleizach 2021-05-20 12:24:40 PDT 
Created attachment 429204 [details]
patch
Comment 6 Per Arne Vollan 2021-05-20 15:12:33 PDT 
Comment on attachment 429204 [details]
patch

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

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1130
> +    auto cfKey = key.createCFString().get();

I think you may need to retain the CFStringRef here.

auto cfKey = key.createCFString();
Comment 7 Per Arne Vollan 2021-05-21 06:24:30 PDT 
Comment on attachment 429204 [details]
patch

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

R=me.

> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1074
> +static const WTF::String& invertColorsPreferenceKey()

Is there an existing constant you can use for this key?
Comment 8 chris fleizach 2021-05-21 10:52:26 PDT 
Comment on attachment 429204 [details]
patch

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

>> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1074
>> +static const WTF::String& invertColorsPreferenceKey()
> 
> Is there an existing constant you can use for this key?

unfortunately no, I can ask to expose one for the future though (Rdar://78317458)

>> Source/WebKit/WebProcess/cocoa/WebProcessCocoa.mm:1130
>> +    auto cfKey = key.createCFString().get();
> 
> I think you may need to retain the CFStringRef here.
> 
> auto cfKey = key.createCFString();

done
Comment 9 chris fleizach 2021-05-21 10:53:24 PDT 
Created attachment 429312 [details]
patch
Comment 10 chris fleizach 2021-05-21 10:54:24 PDT 
Created attachment 429313 [details]
patch
Comment 11 chris fleizach 2021-05-24 09:27:36 PDT 
Created attachment 429538 [details]
patch
Comment 12 EWS 2021-05-24 15:27:10 PDT 
Committed r277971 (238093@main): <https://commits.webkit.org/238093@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 429538 [details].
Comment 13 Tim Horton 2021-05-24 17:53:02 PDT 
Comment on attachment 429538 [details]
patch

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

> Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h:99
> +WK_EXPORT void WKAccessibilityTestingInjectPreference(WKBundlePageRef, const String& domain, const String& key, const Optional<String>& encodedValue);

You can't use WTF::String in API; see WKStringRef. This breaks the build (I'm not sure why EWS didn't catch it, but it's failing for other patches now that it's landed).
Comment 14 chris fleizach 2021-05-24 18:06:48 PDT 
(In reply to Tim Horton from comment #13)
> Comment on attachment 429538 [details]
> patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=429538&action=review
> 
> > Source/WebKit/WebProcess/InjectedBundle/API/c/WKBundlePagePrivate.h:99
> > +WK_EXPORT void WKAccessibilityTestingInjectPreference(WKBundlePageRef, const String& domain, const String& key, const Optional<String>& encodedValue);
> 
> You can't use WTF::String in API; see WKStringRef. This breaks the build
> (I'm not sure why EWS didn't catch it, but it's failing for other patches
> now that it's landed).

Ok will update
Comment 15 WebKit Commit Bot 2021-05-24 18:07:12 PDT 
Re-opened since this is blocked by bug 226202
Comment 16 Truitt Savell 2021-05-28 10:53:31 PDT 
It looks like the changes in https://trac.webkit.org/changeset/277971/webkit

broke TestWebKitAPI.WebKit.AccessibilityReduceMotion and is now timing out

history:
https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebKit.AccessibilityReduceMotion
Comment 17 chris fleizach 2021-05-28 10:57:46 PDT 
(In reply to Truitt Savell from comment #16)
> It looks like the changes in https://trac.webkit.org/changeset/277971/webkit
> 
> broke TestWebKitAPI.WebKit.AccessibilityReduceMotion and is now timing out
> 
> history:
> https://results.webkit.org/?suite=api-tests&test=TestWebKitAPI.WebKit.
> AccessibilityReduceMotion

Thanks for pointing out. Will take a look at that test. Whatever it was checking wasn’t accurate if it was passing before…
Comment 18 Truitt Savell 2021-06-01 10:38:40 PDT 
Filed https://bugs.webkit.org/show_bug.cgi?id=226505