Summary: | Clean up dependencies with accessibility and UIKit | ||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Megan Gardner <megan_gardner> | ||||||||||||||||||||
Component: | New Bugs | Assignee: | Megan Gardner <megan_gardner> | ||||||||||||||||||||
Status: | RESOLVED DUPLICATE | ||||||||||||||||||||||
Severity: | Normal | CC: | aboxhall, apinheiro, benjamin, cdumez, cfleizach, cmarcelo, dbates, dmazzoni, ews-watchlist, jcraig, jdiggs, rniwa, samuel_white, simon.fraser, thorton, wenson_hsieh | ||||||||||||||||||||
Priority: | P2 | ||||||||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||||||
Attachments: |
|
Description
Megan Gardner
2019-04-23 17:52:06 PDT
Created attachment 368093 [details]
Patch
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 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 Created attachment 368096 [details]
Patch
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. Created attachment 368147 [details]
Patch
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 Created attachment 368168 [details]
Patch
Created attachment 368173 [details]
Patch
Created attachment 368179 [details]
Patch
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.
Created attachment 368184 [details]
Patch
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.
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 Created attachment 368253 [details]
Patch
Created attachment 368255 [details]
Patch
https://bugs.webkit.org/show_bug.cgi?id=197279 *** This bug has been marked as a duplicate of bug 197279 *** |