Add the ability to make exclusion rules that exclude elements from being manipulated based on their class.
<rdar://problem/57398802>
Created attachment 384666 [details] 57398802.patch
Created attachment 384723 [details] 57398802-part-2.patch Updated match() function to not mutate the element, and therefore left the argument as const.
Comment on attachment 384723 [details] 57398802-part-2.patch View in context: https://bugs.webkit.org/attachment.cgi?id=384723&action=review > Source/WebCore/ChangeLog:9 > + Tests added as API tests. This should instead be: Tests: TextManipulation.StartTextManipulationApplyInclusionExclusionRulesForClass TextManipulation.StartTextManipulationApplyInclusionExclusionRulesForClassAndAttribute > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:248 > + RetainPtr<_WKTextManipulationConfiguration> configuration = adoptNS([[_WKTextManipulationConfiguration alloc] init]); Why not auto? > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:279 > + RetainPtr<_WKTextManipulationConfiguration> configuration = adoptNS([[_WKTextManipulationConfiguration alloc] init]); Use auto?
Comment on attachment 384723 [details] 57398802-part-2.patch View in context: https://bugs.webkit.org/attachment.cgi?id=384723&action=review > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:283 > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forAttribute:@"data-exclude" value:@"yes"] autorelease], > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)NO forAttribute:@"data-exclude" value:@"no"] autorelease], > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forClass:@"exclude"] autorelease], Nit - I don’t think you need the (BOOL) casts here.
(In reply to Ryosuke Niwa from comment #4) > Comment on attachment 384723 [details] > 57398802-part-2.patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384723&action=review > > > Source/WebCore/ChangeLog:9 > > + Tests added as API tests. > > This should instead be: > Tests: > TextManipulation.StartTextManipulationApplyInclusionExclusionRulesForClass > > TextManipulation. > StartTextManipulationApplyInclusionExclusionRulesForClassAndAttribute Fixed. > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:248 > > + RetainPtr<_WKTextManipulationConfiguration> configuration = adoptNS([[_WKTextManipulationConfiguration alloc] init]); > > Why not auto? No specific reason, I'm just in the habit of declaring types since I'm used to Objective-C. I'll make these `auto`. > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:279 > > + RetainPtr<_WKTextManipulationConfiguration> configuration = adoptNS([[_WKTextManipulationConfiguration alloc] init]); > > Use auto?
(In reply to Wenson Hsieh from comment #5) > Comment on attachment 384723 [details] > 57398802-part-2.patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=384723&action=review > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:283 > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forAttribute:@"data-exclude" value:@"yes"] autorelease], > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)NO forAttribute:@"data-exclude" value:@"no"] autorelease], > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forClass:@"exclude"] autorelease], > > Nit - I don’t think you need the (BOOL) casts here. At one point I had a build issue where it didn't understand these values, and a typecast fixed it. I'll see if these are still required.
(In reply to Louie Livon-Bemel from comment #7) > (In reply to Wenson Hsieh from comment #5) > > Comment on attachment 384723 [details] > > 57398802-part-2.patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=384723&action=review > > > > > Tools/TestWebKitAPI/Tests/WebKitCocoa/TextManipulation.mm:283 > > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forAttribute:@"data-exclude" value:@"yes"] autorelease], > > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)NO forAttribute:@"data-exclude" value:@"no"] autorelease], > > > + [[[_WKTextManipulationExclusionRule alloc] initExclusion:(BOOL)YES forClass:@"exclude"] autorelease], > > > > Nit - I don’t think you need the (BOOL) casts here. > > At one point I had a build issue where it didn't understand these values, > and a typecast fixed it. I'll see if these are still required. Testing this now, I see no issues when removing the typecast. It looks like the tests Ryosuke originally added include the typecast so that's probably why I did too. I'll remove them from my tests though.
Created attachment 384820 [details] 57398802-part-3.patch
Comment on attachment 384820 [details] 57398802-part-3.patch Clearing flags on attachment: 384820 Committed r253112: <https://trac.webkit.org/changeset/253112>
All reviewed patches have been landed. Closing bug.