.
<rdar://problem/107335186>
Created attachment 465643 [details] Patch
Comment on attachment 465643 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=465643&action=review > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3583 > + unsigned length = Accessibility::retrieveValueFromMainThread<int>([textMarkerRange = retainPtr(textMarkerRange), protectedSelf = retainPtr(self)] () -> int { This declares that it returns `-> int`, but it now should be `-> unsigned`, right? > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3591 > - if (length < 0) > - return nil; > return @(length); Prior to your patch, we would return -1 for std::nullopt ranges. Now that `lengthForRange` returns unsigned, we will never return `nil` for this attribute. Is this behavior change OK?
Created attachment 465648 [details] Patch
(In reply to Tyler Wilcock from comment #3) > Comment on attachment 465643 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=465643&action=review > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3583 > > + unsigned length = Accessibility::retrieveValueFromMainThread<int>([textMarkerRange = retainPtr(textMarkerRange), protectedSelf = retainPtr(self)] () -> int { > > This declares that it returns `-> int`, but it now should be `-> unsigned`, > right? Yes, fixed. > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3591 > > - if (length < 0) > > - return nil; > > return @(length); > > Prior to your patch, we would return -1 for std::nullopt ranges. Now that > `lengthForRange` returns unsigned, we will never return `nil` for this > attribute. Is this behavior change OK? I believe so, no client should expect a -1 length.
(In reply to Andres Gonzalez from comment #5) > (In reply to Tyler Wilcock from comment #3) > > Comment on attachment 465643 [details] > > Patch > > > > View in context: > > https://bugs.webkit.org/attachment.cgi?id=465643&action=review > > > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3583 > > > + unsigned length = Accessibility::retrieveValueFromMainThread<int>([textMarkerRange = retainPtr(textMarkerRange), protectedSelf = retainPtr(self)] () -> int { > > > > This declares that it returns `-> int`, but it now should be `-> unsigned`, > > right? > > Yes, fixed. > > > > > Source/WebCore/accessibility/mac/WebAccessibilityObjectWrapperMac.mm:3591 > > > - if (length < 0) > > > - return nil; > > > return @(length); > > > > Prior to your patch, we would return -1 for std::nullopt ranges. Now that > > `lengthForRange` returns unsigned, we will never return `nil` for this > > attribute. Is this behavior change OK? > > I believe so, no client should expect a -1 length. or rather a nil when requesting the length of a range.
Committed 262270@main (3a1d73db387c): <https://commits.webkit.org/262270@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 465648 [details].