Bug 197222 – Clean up dependencies with accessibility and UIKit

• Component: New Bugs
• Status: RESOLVED DUPLICATE

[Megan Gardner]

Clean up dependencies with accessibility and UIKit

[Megan Gardner]

Created attachment 368093 [details]
Patch

[chris fleizach]

Comment on attachment 368093 [details]
Patch

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

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1184
>  {

I don’t think this change is correct. Ax is expecting this method name

[Tim Horton]

Comment on attachment 368093 [details]
Patch

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

>> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1184
>>  {
> 
> I don’t think this change is correct. Ax is expecting this method name

Yeah

[Megan Gardner]

Created attachment 368096 [details]
Patch

[Simon Fraser (smfr)]

Comment on attachment 368096 [details]
Patch

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

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:1074
> +#if HAVE_ACCESSIBILITY_DEFINED_IN_UIKIT

It's odd to use the term "defined" here (too easily confused with preprocessor macro defining). If it's only about UIAccessibilityScrollDirection and NSAttachmentCharacter, maybe there should be two HAVE_ with better names.

[Megan Gardner]

Created attachment 368147 [details]
Patch

[chris fleizach]

Comment on attachment 368147 [details]
Patch

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

> Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1184
> +#pragma clang diagnostic ignored "-Wdeprecated-implementations"

whats the warning that gets output? the actual header says

@property(nullable, nonatomic, copy) NSArray *accessibilityHeaderElements UIKIT_AVAILABLE_TVOS_ONLY(9_0);

but it shouldn't stop us from implementing it right?

> Source/WebKit/Platform/spi/ios/UIKitSPI.h:1087
> +static NSString * const UIAccessibilityTextAttributeContext = @"UIAccessibilityTextAttributeContext";

these are in iOS13 so we could also just do a > IOS13 #if where we use them

[attrString addAttribute:PAL::get_UIKit_UIAccessibilityTextAttributeContext() value:PAL::get_UIKit_UIAccessibilityTextualContextSourceCode() range:range];

> Tools/ChangeLog:10
> +        * DumpRenderTree/ios/AccessibilityUIElementIOS.mm:

where there any tool changes, not seeing them here

[Megan Gardner]

Created attachment 368168 [details]
Patch

[Megan Gardner]

Created attachment 368173 [details]
Patch

[Megan Gardner]

Created attachment 368179 [details]
Patch

[EWS Watchlist]

Attachment 368179 [details] did not pass style-queue:


ERROR: Source/WebKit/Platform/spi/ios/UIKitSPI.h:1073:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebKit/Platform/spi/ios/UIKitSPI.h:1092:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Megan Gardner]

Created attachment 368184 [details]
Patch

[EWS Watchlist]

Attachment 368184 [details] did not pass style-queue:


ERROR: Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h:150:  Should have a space between // and comment  [whitespace/comments] [4]
ERROR: Source/WebCore/PAL/pal/spi/ios/UIKitSPI.h:169:  Should have a space between // and comment  [whitespace/comments] [4]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[chris fleizach]

Don't feel like current patch addresses any of this feedback

(In reply to chris fleizach from comment #7)
> Comment on attachment 368147 [details]
> Patch
> 
> View in context:
> https://bugs.webkit.org/attachment.cgi?id=368147&action=review
> 
> > Source/WebCore/accessibility/ios/WebAccessibilityObjectWrapperIOS.mm:1184
> > +#pragma clang diagnostic ignored "-Wdeprecated-implementations"
> 
> whats the warning that gets output? the actual header says
> 
> @property(nullable, nonatomic, copy) NSArray *accessibilityHeaderElements
> UIKIT_AVAILABLE_TVOS_ONLY(9_0);
> 
> but it shouldn't stop us from implementing it right?
> 
> > Source/WebKit/Platform/spi/ios/UIKitSPI.h:1087
> > +static NSString * const UIAccessibilityTextAttributeContext = @"UIAccessibilityTextAttributeContext";
> 
> these are in iOS13 so we could also just do a > IOS13 #if where we use them
> 
> [attrString
> addAttribute:PAL::get_UIKit_UIAccessibilityTextAttributeContext()
> value:PAL::get_UIKit_UIAccessibilityTextualContextSourceCode() range:range];
> 
> > Tools/ChangeLog:10
> > +        * DumpRenderTree/ios/AccessibilityUIElementIOS.mm:
> 
> where there any tool changes, not seeing them here

[Megan Gardner]

Created attachment 368253 [details]
Patch

[Megan Gardner]

Created attachment 368255 [details]
Patch

[Megan Gardner]

https://bugs.webkit.org/show_bug.cgi?id=197279

*** This bug has been marked as a duplicate of bug 197279 ***