This function is currently: String AccessibilityObject::listMarkerTextForNodeAndPosition(Node* node, const VisiblePosition& visiblePositionStart) { // If the range does not contain the start of the line, the list marker text should not be included. if (!isStartOfLine(visiblePositionStart)) return String(); // We should speak the list marker only for the first line. RenderListItem* listItem = renderListItemContainerForNode(node); if (!listItem) return String(); if (!inSameLine(visiblePositionStart, firstPositionInNode(&listItem->element()))) return String(); return listMarkerTextForNode(node); } The !isStartOfLine(visiblePositionStart) check is redundant, covered entirely by the later !inSameLine(visiblePositionStart, firstPositionInNode(&listItem->element())) check.
<rdar://problem/90781742>
Created attachment 455660 [details] Patch
(In reply to Tyler Wilcock from comment #2) > Created attachment 455660 [details] > Patch --- a/Source/WebCore/ChangeLog +++ a/Source/WebCore/ChangeLog + In AccessibilityObject::listMarkerTextForNodeAndPosition, the !isStartOfLine(visiblePositionStart) + check is redundant, covered entirely by the later + !inSameLine(visiblePositionStart, + firstPositionInNode(&listItem->element())) check. This patch removes + the former check, since it can sometimes cause crashes. --- a/Source/WebCore/accessibility/AccessibilityObject.cpp +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp @@ -1455,17 +1455,13 @@ static String listMarkerTextForNode(Node* node) // Returns the text associated with a list marker if this node is contained within a list item. String AccessibilityObject::listMarkerTextForNodeAndPosition(Node* node, const VisiblePosition& visiblePositionStart) { - // If the range does not contain the start of the line, the list marker text should not be included. - if (!isStartOfLine(visiblePositionStart)) - return String(); I don't see how the inSameLine check includes the above check inSameLine checks whether the two positions have the same startOfLine, but the above check is whether the given position is exactly startOfLine.
(In reply to Andres Gonzalez from comment #3) > (In reply to Tyler Wilcock from comment #2) > > Created attachment 455660 [details] > > Patch > > --- a/Source/WebCore/ChangeLog > +++ a/Source/WebCore/ChangeLog > > + In AccessibilityObject::listMarkerTextForNodeAndPosition, the > !isStartOfLine(visiblePositionStart) > + check is redundant, covered entirely by the later > + !inSameLine(visiblePositionStart, > + firstPositionInNode(&listItem->element())) check. This patch removes > + the former check, since it can sometimes cause crashes. > > --- a/Source/WebCore/accessibility/AccessibilityObject.cpp > +++ a/Source/WebCore/accessibility/AccessibilityObject.cpp > @@ -1455,17 +1455,13 @@ static String listMarkerTextForNode(Node* node) > // Returns the text associated with a list marker if this node is contained > within a list item. > String AccessibilityObject::listMarkerTextForNodeAndPosition(Node* node, > const VisiblePosition& visiblePositionStart) > { > - // If the range does not contain the start of the line, the list marker > text should not be included. > - if (!isStartOfLine(visiblePositionStart)) > - return String(); > > I don't see how the inSameLine check includes the above check inSameLine > checks whether the two positions have the same startOfLine, but the above > check is whether the given position is exactly startOfLine. Perhaps we can try: auto* listItem = renderListItemContainerForNode(node); if (!listItem) return { }; if (!isStartOfLine(visiblePositionStart) || !inSameLine(visiblePositionStart, firstPositionInNode(&listItem->element()))) return { };
Created attachment 455764 [details] Patch
Committed r291968 (248933@main): <https://commits.webkit.org/248933@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455764 [details].