Bug 225909 - AX: Settings: Increase contrast isn't detected in browser until an additional setting is applied
Summary: AX: Settings: Increase contrast isn't detected in browser until an additional...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Accessibility (show other bugs)
Version: WebKit Nightly Build
Hardware: All All
: P2 Normal
Assignee: chris fleizach
URL:
Keywords: InRadar
Depends on: 226202
Blocks:
  Show dependency tree / graph
 
Reported: 2021-05-18 00:54 PDT by chris fleizach
Modified: 2021-06-02 11:59 PDT (History)
17 users (show)

See Also:

Attachments
patch (15.83 KB, patch)
2021-05-20 00:37 PDT, chris fleizach 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (18.72 KB, patch)
2021-05-20 00:55 PDT, chris fleizach 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (19.32 KB, patch)
2021-05-20 10:52 PDT, chris fleizach 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (19.33 KB, patch)
2021-05-20 11:00 PDT, chris fleizach 		ews-feeder: commit-queue-
Details | Formatted Diff | Diff
patch (19.30 KB, patch)
2021-05-20 12:24 PDT, chris fleizach 		pvollan: review+
Details | Formatted Diff | Diff
patch (16.92 KB, patch)
2021-05-21 10:53 PDT, chris fleizach 		no flags Details | Formatted Diff | Diff
patch (14.54 KB, patch)
2021-05-21 10:54 PDT, chris fleizach 		no flags Details | Formatted Diff | Diff
patch (21.68 KB, patch)
2021-05-24 09:27 PDT, chris fleizach 		no flags Details | Formatted Diff | Diff
Show Obsolete (7) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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