From 2f2e5d80a28e745267941ffb660f2ca768c0c0b6 Mon Sep 17 00:00:00 2001 From: James Teh Date: Sat, 26 Feb 2022 23:01:56 +0000 Subject: [PATCH] Bug 1756528: Fix cropping of siblings in TextRange::Crop. r=eeejay There was already a code path to handle siblings, but this only applied if the boundary child at the range's start/end (often a text leaf) was a sibling of aContainer. It didn't apply if aContainer was a direct sibling of the range's start/end container. To fix this, don't restrict the code which handles the case where aContainer does not contain the start/end boundary. This should always fail to crop, regardless of the ancestry. Differential Revision: https://phabricator.services.mozilla.com/D139351 --- accessible/base/TextRange.cpp | 4 ++-- .../browser/e10s/browser_caching_text.js | 21 ++++++++++++++++++- .../mochitest/textrange/test_selection.html | 15 +++++++++++++ 3 files changed, 37 insertions(+), 3 deletions(-) diff --git a/accessible/base/TextRange.cpp b/accessible/base/TextRange.cpp index 3bcbaed2ff6b..75f91f4932a4 100644 --- a/accessible/base/TextRange.cpp +++ b/accessible/base/TextRange.cpp @@ -203,7 +203,7 @@ bool TextRange::Crop(Accessible* aContainer) { // The start boundary and the container are siblings. container = aContainer; } - } else if (containerPos != 0) { + } else { // The container does not contain the start boundary. boundary = boundaryParents[boundaryPos]; container = containerParents[containerPos]; @@ -243,7 +243,7 @@ bool TextRange::Crop(Accessible* aContainer) { } else { container = aContainer; } - } else if (containerPos != 0) { + } else { boundary = boundaryParents[boundaryPos]; container = containerParents[containerPos]; } diff --git a/accessible/tests/browser/e10s/browser_caching_text.js b/accessible/tests/browser/e10s/browser_caching_text.js index 5c462438be17..4d79fffffcc8 100644 --- a/accessible/tests/browser/e10s/browser_caching_text.js +++ b/accessible/tests/browser/e10s/browser_caching_text.js @@ -819,8 +819,16 @@ addAccessibleTask( testSelectionRange(browser, editable, p1, 0, p1, 1); testTextGetSelection(editable, 0, 1, 0); testTextGetSelection(p1, 0, 1, 0); - const p2 = findAccessibleChildByID(docAcc, "p2", [nsIAccessibleText]); + if (isCacheEnabled && browser.isRemoteBrowser) { + is(p2.selectionCount, 0, "p2 selectionCount is 0"); + } else { + todo( + false, + "Siblings report wrong selection in non-cache implementation" + ); + } + // Selecting across two Accessibles with only a partial selection in the // second. info("Selecting ab in editable"); @@ -899,6 +907,17 @@ addAccessibleTask( is(editable.selectionCount, 2, "editable selectionCount is 2"); testTextGetSelection(editable, 0, 1, 0); testTextGetSelection(editable, 1, 2, 1); + if (isCacheEnabled && browser.isRemoteBrowser) { + is(p1.selectionCount, 1, "p1 selectionCount is 1"); + testTextGetSelection(p1, 0, 1, 0); + is(p2.selectionCount, 1, "p2 selectionCount is 1"); + testTextGetSelection(p2, 0, 1, 0); + } else { + todo( + false, + "Siblings report wrong selection in non-cache implementation" + ); + } }, { chrome: true, diff --git a/accessible/tests/mochitest/textrange/test_selection.html b/accessible/tests/mochitest/textrange/test_selection.html index fb1c0884df1d..2a5d4da5c23d 100644 --- a/accessible/tests/mochitest/textrange/test_selection.html +++ b/accessible/tests/mochitest/textrange/test_selection.html @@ -105,6 +105,19 @@ res = a11yrange.compareEndPoints(EndPoint_End, a11yrange, EndPoint_Start); is(res, 1, "end must be greater than start"); + // Crop a range to its next sibling. + range.selectNode(getNode("c3p1").firstChild); + a11yranges = getAccessible(document, [nsIAccessibleText]).selectionRanges; + a11yrange = a11yranges.queryElementAt(0, nsIAccessibleTextRange); + testTextRange(a11yrange, "selection range #8", "c3p1", 0, "c3p1", 1); + ok(!a11yrange.crop(getAccessible("c3p2")), "Crop #8 succeeded but shouldn't have."); + // Crop a range to its previous sibling. + range.selectNode(getNode("c3p2").firstChild); + a11yranges = getAccessible(document, [nsIAccessibleText]).selectionRanges; + a11yrange = a11yranges.queryElementAt(0, nsIAccessibleTextRange); + testTextRange(a11yrange, "selection range #9", "c3p2", 0, "c3p2", 1); + ok(!a11yrange.crop(getAccessible("c3p1")), "Crop #9 succeeded but shouldn't have."); + SimpleTest.finish(); } @@ -125,5 +138,7 @@

text link text

start
cell
end
+ +

a

b