238341 2022-03-24 11:42:35 -0700 AccessibilityObject::listMarkerTextForNodeAndPosition should check for presence of list item before anything else 2022-03-28 09:30:15 -0700 1 1 1 Unclassified WebKit Accessibility WebKit Nightly Build All All RESOLVED FIXED InRadar P2 Normal --- 1 tyler_w tyler_w aboxhall andresg_22 apinheiro cfleizach dmazzoni ews-watchlist jcraig jdiggs samuel_white webkit-bug-importer oldest_to_newest 1854789 0 tyler_w 2022-03-24 11:42:35 -0700 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. 1854790 1 webkit-bug-importer 2022-03-24 11:42:46 -0700 <rdar://problem/90781742> 1854791 2 455660 tyler_w 2022-03-24 11:45:13 -0700 Created attachment 455660 Patch 1855068 3 andresg_22 2022-03-25 05:55:25 -0700 (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. 1855074 4 andresg_22 2022-03-25 06:06:07 -0700 (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 { }; 1855094 5 455764 tyler_w 2022-03-25 08:31:29 -0700 Created attachment 455764 Patch 1855638 6 ews-feeder 2022-03-28 09:30:12 -0700 Committed r291968 (248933@main): <https://commits.webkit.org/248933@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 455764. 455660 2022-03-24 11:45:13 -0700 2022-03-25 08:31:25 -0700 Patch bug-238341-20220324134512.patch text/plain 2625 tyler_w U3VidmVyc2lvbiBSZXZpc2lvbjogMjkxNjAyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjk3ZDQ3MDIyZjNjMWYz YzRkY2I5MzE5MDE1MTg5NzdiN2ZkYjNjYy4uY2ViMDViZTI1NTA3ZWJjOGE4NTU2Y2M0MTVjMDQw ZmRkNjJiMGU5MSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIyIEBACisyMDIyLTAzLTI0ICBUeWxl ciBXaWxjb2NrICA8dHlsZXJfd0BhcHBsZS5jb20+CisKKyAgICAgICAgUmVtb3ZlIHJlZHVuZGFu dCBpc1N0YXJ0T2ZMaW5lIGNoZWNrIGluIEFjY2Vzc2liaWxpdHlPYmplY3Q6Omxpc3RNYXJrZXJU ZXh0Rm9yTm9kZUFuZFBvc2l0aW9uCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3No b3dfYnVnLmNnaT9pZD0yMzgzNDEKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMh KS4KKworICAgICAgICBJbiBBY2Nlc3NpYmlsaXR5T2JqZWN0OjpsaXN0TWFya2VyVGV4dEZvck5v ZGVBbmRQb3NpdGlvbiwgdGhlICFpc1N0YXJ0T2ZMaW5lKHZpc2libGVQb3NpdGlvblN0YXJ0KQor ICAgICAgICBjaGVjayBpcyByZWR1bmRhbnQsIGNvdmVyZWQgZW50aXJlbHkgYnkgdGhlIGxhdGVy CisgICAgICAgICFpblNhbWVMaW5lKHZpc2libGVQb3NpdGlvblN0YXJ0LAorICAgICAgICBmaXJz dFBvc2l0aW9uSW5Ob2RlKCZsaXN0SXRlbS0+ZWxlbWVudCgpKSkgY2hlY2suIFRoaXMgcGF0Y2gg cmVtb3ZlcworICAgICAgICB0aGUgZm9ybWVyIGNoZWNrLCBzaW5jZSBpdCBjYW4gc29tZXRpbWVz IGNhdXNlIGNyYXNoZXMuCisKKyAgICAgICAgQ292ZXJlZCBieSB0ZXN0CisgICAgICAgIGFjY2Vz c2liaWxpdHkvbWFjL2F0dHJpYnV0ZWQtc3RyaW5nLXdpdGgtbGlzdGl0ZW0tbXVsdGlwbGUtbGlu ZXMuaHRtbC4KKworICAgICAgICAqIGFjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU9iamVjdC5j cHA6CisgICAgICAgIChXZWJDb3JlOjpBY2Nlc3NpYmlsaXR5T2JqZWN0OjpsaXN0TWFya2VyVGV4 dEZvck5vZGVBbmRQb3NpdGlvbik6CisKIDIwMjItMDMtMjEgIFR5bGVyIFdpbGNvY2sgIDx0eWxl cl93QGFwcGxlLmNvbT4KIAogICAgICAgICBBWDogQWNjZXNzaWJpbGl0eU9iamVjdDo6dmlzaWJs ZUNoYXJhY3RlclJhbmdlIGlzIGV4dHJlbWVseSBzbG93IHdoZW4gY2FsbGVkIG9uIG9iamVjdHMg d2l0aCBsb3RzIG9mIHRleHQKZGlmZiAtLWdpdCBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxp dHkvQWNjZXNzaWJpbGl0eU9iamVjdC5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmlsaXR5 L0FjY2Vzc2liaWxpdHlPYmplY3QuY3BwCmluZGV4IDVhZDIyYzE3MGY1NDI3YTg1Yjc1NDgzNzg5 MDVkYzBmMjdlMmViZjIuLjAzMzgzMWM3NjI5ZDUzNDQ5MTRiODk0ZjFmN2Q4ZDM0N2U0NjJmMWQg MTAwNjQ0Ci0tLSBhL1NvdXJjZS9XZWJDb3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU9i amVjdC5jcHAKKysrIGIvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0eS9BY2Nlc3NpYmlsaXR5 T2JqZWN0LmNwcApAQCAtMTQ1NSwxNyArMTQ1NSwxMyBAQCBzdGF0aWMgU3RyaW5nIGxpc3RNYXJr ZXJUZXh0Rm9yTm9kZShOb2RlKiBub2RlKQogLy8gUmV0dXJucyB0aGUgdGV4dCBhc3NvY2lhdGVk IHdpdGggYSBsaXN0IG1hcmtlciBpZiB0aGlzIG5vZGUgaXMgY29udGFpbmVkIHdpdGhpbiBhIGxp c3QgaXRlbS4KIFN0cmluZyBBY2Nlc3NpYmlsaXR5T2JqZWN0OjpsaXN0TWFya2VyVGV4dEZvck5v ZGVBbmRQb3NpdGlvbihOb2RlKiBub2RlLCBjb25zdCBWaXNpYmxlUG9zaXRpb24mIHZpc2libGVQ b3NpdGlvblN0YXJ0KQogewotICAgIC8vIElmIHRoZSByYW5nZSBkb2VzIG5vdCBjb250YWluIHRo ZSBzdGFydCBvZiB0aGUgbGluZSwgdGhlIGxpc3QgbWFya2VyIHRleHQgc2hvdWxkIG5vdCBiZSBp bmNsdWRlZC4KLSAgICBpZiAoIWlzU3RhcnRPZkxpbmUodmlzaWJsZVBvc2l0aW9uU3RhcnQpKQot ICAgICAgICByZXR1cm4gU3RyaW5nKCk7Ci0KLSAgICAvLyBXZSBzaG91bGQgc3BlYWsgdGhlIGxp c3QgbWFya2VyIG9ubHkgZm9yIHRoZSBmaXJzdCBsaW5lLgotICAgIFJlbmRlckxpc3RJdGVtKiBs aXN0SXRlbSA9IHJlbmRlckxpc3RJdGVtQ29udGFpbmVyRm9yTm9kZShub2RlKTsKKyAgICBhdXRv KiBsaXN0SXRlbSA9IHJlbmRlckxpc3RJdGVtQ29udGFpbmVyRm9yTm9kZShub2RlKTsKICAgICBp ZiAoIWxpc3RJdGVtKQogICAgICAgICByZXR1cm4gU3RyaW5nKCk7CisgICAgLy8gSWYgdGhlIHJh bmdlIGRvZXMgbm90IG92ZXJsYXAgd2l0aCB0aGUgc3RhcnQgb2YgdGhlIGxpc3QgaXRlbSAod2hl cmUgdGhlIG1hcmtlciBpcyksIHRoZSBsaXN0IG1hcmtlciB0ZXh0IHNob3VsZCBub3QgYmUgaW5j bHVkZWQuCiAgICAgaWYgKCFpblNhbWVMaW5lKHZpc2libGVQb3NpdGlvblN0YXJ0LCBmaXJzdFBv c2l0aW9uSW5Ob2RlKCZsaXN0SXRlbS0+ZWxlbWVudCgpKSkpCiAgICAgICAgIHJldHVybiBTdHJp bmcoKTsKLSAgICAKKwogICAgIHJldHVybiBsaXN0TWFya2VyVGV4dEZvck5vZGUobm9kZSk7CiB9 CiAK 455764 2022-03-25 08:31:29 -0700 2022-03-28 09:30:13 -0700 Patch bug-238341-20220325103127.patch text/plain 2758 tyler_w U3VidmVyc2lvbiBSZXZpc2lvbjogMjkxNjAyCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNjk3ZDQ3MDIyZjNjMWYz YzRkY2I5MzE5MDE1MTg5NzdiN2ZkYjNjYy4uMTUyYzBkYzA0OWYzNTNkODY0NjFmNGZkOTA5MTc4 NWE2NjdiN2Y4ZiAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDIxIEBACisyMDIyLTAzLTI0ICBUeWxl ciBXaWxjb2NrICA8dHlsZXJfd0BhcHBsZS5jb20+CisKKyAgICAgICAgQWNjZXNzaWJpbGl0eU9i amVjdDo6bGlzdE1hcmtlclRleHRGb3JOb2RlQW5kUG9zaXRpb24gc2hvdWxkIGNoZWNrIGZvciBw cmVzZW5jZSBvZiBsaXN0IGl0ZW0gYmVmb3JlIGFueXRoaW5nIGVsc2UKKyAgICAgICAgaHR0cHM6 Ly9idWdzLndlYmtpdC5vcmcvc2hvd19idWcuY2dpP2lkPTIzODM0MQorCisgICAgICAgIFJldmll d2VkIGJ5IE5PQk9EWSAoT09QUyEpLgorCisgICAgICAgIFRoZSBmaXJzdCB0aGluZyBBY2Nlc3Np YmlsaXR5T2JqZWN0OjpsaXN0TWFya2VyVGV4dEZvck5vZGVBbmRQb3NpdGlvbgorICAgICAgICBk b2VzIGlzIGNoZWNrIHRvIHNlZSB0aGF0IHRoZSBnaXZlbiByYW5nZSBgaXNTdGFydE9mTGluZWAu IFdlCisgICAgICAgIHNob3VsZCBpbnN0ZWFkIGNoZWNrIGlmIHRoZXJlJ3MgYW4gYWN0dWFsIGxp c3QgaXRlbSB0byB3b3JrIHdpdGgKKyAgICAgICAgYmVmb3JlIGRvaW5nIHRoaXMsIHNpbmNlIGBp c1N0YXJ0T2ZMaW5lYCBjYW4gY2F1c2UgY3Jhc2hlcy4KKworICAgICAgICBDb3ZlcmVkIGJ5IHRl c3QKKyAgICAgICAgYWNjZXNzaWJpbGl0eS9tYWMvYXR0cmlidXRlZC1zdHJpbmctd2l0aC1saXN0 aXRlbS1tdWx0aXBsZS1saW5lcy5odG1sLgorCisgICAgICAgICogYWNjZXNzaWJpbGl0eS9BY2Nl c3NpYmlsaXR5T2JqZWN0LmNwcDoKKyAgICAgICAgKFdlYkNvcmU6OkFjY2Vzc2liaWxpdHlPYmpl Y3Q6Omxpc3RNYXJrZXJUZXh0Rm9yTm9kZUFuZFBvc2l0aW9uKToKKwogMjAyMi0wMy0yMSAgVHls ZXIgV2lsY29jayAgPHR5bGVyX3dAYXBwbGUuY29tPgogCiAgICAgICAgIEFYOiBBY2Nlc3NpYmls aXR5T2JqZWN0Ojp2aXNpYmxlQ2hhcmFjdGVyUmFuZ2UgaXMgZXh0cmVtZWx5IHNsb3cgd2hlbiBj YWxsZWQgb24gb2JqZWN0cyB3aXRoIGxvdHMgb2YgdGV4dApkaWZmIC0tZ2l0IGEvU291cmNlL1dl YkNvcmUvYWNjZXNzaWJpbGl0eS9BY2Nlc3NpYmlsaXR5T2JqZWN0LmNwcCBiL1NvdXJjZS9XZWJD b3JlL2FjY2Vzc2liaWxpdHkvQWNjZXNzaWJpbGl0eU9iamVjdC5jcHAKaW5kZXggNWFkMjJjMTcw ZjU0MjdhODViNzU0ODM3ODkwNWRjMGYyN2UyZWJmMi4uNzU2MWRlM2E1Njc4YWYzYWQ1N2Q4YTY5 YjM4NDE0ZmEyZGY1ZTk0ZCAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvYWNjZXNzaWJpbGl0 eS9BY2Nlc3NpYmlsaXR5T2JqZWN0LmNwcAorKysgYi9Tb3VyY2UvV2ViQ29yZS9hY2Nlc3NpYmls aXR5L0FjY2Vzc2liaWxpdHlPYmplY3QuY3BwCkBAIC0xNDU1LDE3ICsxNDU1LDEzIEBAIHN0YXRp YyBTdHJpbmcgbGlzdE1hcmtlclRleHRGb3JOb2RlKE5vZGUqIG5vZGUpCiAvLyBSZXR1cm5zIHRo ZSB0ZXh0IGFzc29jaWF0ZWQgd2l0aCBhIGxpc3QgbWFya2VyIGlmIHRoaXMgbm9kZSBpcyBjb250 YWluZWQgd2l0aGluIGEgbGlzdCBpdGVtLgogU3RyaW5nIEFjY2Vzc2liaWxpdHlPYmplY3Q6Omxp c3RNYXJrZXJUZXh0Rm9yTm9kZUFuZFBvc2l0aW9uKE5vZGUqIG5vZGUsIGNvbnN0IFZpc2libGVQ b3NpdGlvbiYgdmlzaWJsZVBvc2l0aW9uU3RhcnQpCiB7Ci0gICAgLy8gSWYgdGhlIHJhbmdlIGRv ZXMgbm90IGNvbnRhaW4gdGhlIHN0YXJ0IG9mIHRoZSBsaW5lLCB0aGUgbGlzdCBtYXJrZXIgdGV4 dCBzaG91bGQgbm90IGJlIGluY2x1ZGVkLgotICAgIGlmICghaXNTdGFydE9mTGluZSh2aXNpYmxl UG9zaXRpb25TdGFydCkpCi0gICAgICAgIHJldHVybiBTdHJpbmcoKTsKLQotICAgIC8vIFdlIHNo b3VsZCBzcGVhayB0aGUgbGlzdCBtYXJrZXIgb25seSBmb3IgdGhlIGZpcnN0IGxpbmUuCi0gICAg UmVuZGVyTGlzdEl0ZW0qIGxpc3RJdGVtID0gcmVuZGVyTGlzdEl0ZW1Db250YWluZXJGb3JOb2Rl KG5vZGUpOworICAgIGF1dG8qIGxpc3RJdGVtID0gcmVuZGVyTGlzdEl0ZW1Db250YWluZXJGb3JO b2RlKG5vZGUpOwogICAgIGlmICghbGlzdEl0ZW0pCi0gICAgICAgIHJldHVybiBTdHJpbmcoKTsK LSAgICBpZiAoIWluU2FtZUxpbmUodmlzaWJsZVBvc2l0aW9uU3RhcnQsIGZpcnN0UG9zaXRpb25J bk5vZGUoJmxpc3RJdGVtLT5lbGVtZW50KCkpKSkKLSAgICAgICAgcmV0dXJuIFN0cmluZygpOwot ICAgIAorICAgICAgICByZXR1cm4geyB9OworICAgIC8vIE9ubHkgaW5jbHVkZSB0aGUgbGlzdCBt YXJrZXIgaWYgdGhlIHJhbmdlIGluY2x1ZGVzIHRoZSBsaW5lIHN0YXJ0ICh3aGVyZSB0aGUgbWFy a2VyIHdvdWxkIGJlKSwgYW5kIGlzIGluIHRoZSBzYW1lIGxpbmUgYXMgdGhlIG1hcmtlci4KKyAg ICBpZiAoIWlzU3RhcnRPZkxpbmUodmlzaWJsZVBvc2l0aW9uU3RhcnQpIHx8ICFpblNhbWVMaW5l KHZpc2libGVQb3NpdGlvblN0YXJ0LCBmaXJzdFBvc2l0aW9uSW5Ob2RlKCZsaXN0SXRlbS0+ZWxl bWVudCgpKSkpCisgICAgICAgIHJldHVybiB7IH07CisKICAgICByZXR1cm4gbGlzdE1hcmtlclRl eHRGb3JOb2RlKG5vZGUpOwogfQogCg==