Bug 1969004 - CSS Highlight API: Make Highlight.add() and HighlightRegistry.set() chainable. a=dmeehan

Original Revision: https://phabricator.services.mozilla.com/D251599

Differential Revision: https://phabricator.services.mozilla.com/D252466
This commit is contained in:
Jan-Niklas Jaeschke
2025-06-05 13:31:01 +00:00
committed by dmeehan@mozilla.com
parent 5917881724
commit e6f31faf20
7 changed files with 29 additions and 13 deletions

View File

@@ -125,7 +125,7 @@ void Highlight::SetType(HighlightType aHighlightType) {
Repaint(); Repaint();
} }
void Highlight::Add(AbstractRange& aRange, ErrorResult& aRv) { Highlight* Highlight::Add(AbstractRange& aRange, ErrorResult& aRv) {
// Manually check if the range `aKey` is already present in this highlight, // Manually check if the range `aKey` is already present in this highlight,
// because `SetlikeHelpers::Add()` doesn't indicate this. // because `SetlikeHelpers::Add()` doesn't indicate this.
// To keep the setlike and the mirrored array in sync, the range must not // To keep the setlike and the mirrored array in sync, the range must not
@@ -134,11 +134,11 @@ void Highlight::Add(AbstractRange& aRange, ErrorResult& aRv) {
// `nsTArray<>::Contains()`. // `nsTArray<>::Contains()`.
if (Highlight_Binding::SetlikeHelpers::Has(this, aRange, aRv) || if (Highlight_Binding::SetlikeHelpers::Has(this, aRange, aRv) ||
aRv.Failed()) { aRv.Failed()) {
return; return this;
} }
Highlight_Binding::SetlikeHelpers::Add(this, aRange, aRv); Highlight_Binding::SetlikeHelpers::Add(this, aRange, aRv);
if (aRv.Failed()) { if (aRv.Failed()) {
return; return this;
} }
MOZ_ASSERT(!mRanges.Contains(&aRange), MOZ_ASSERT(!mRanges.Contains(&aRange),
@@ -155,9 +155,10 @@ void Highlight::Add(AbstractRange& aRange, ErrorResult& aRv) {
// no strong reference is needed to keep `registry` alive. // no strong reference is needed to keep `registry` alive.
MOZ_KnownLive(registry)->MaybeAddRangeToHighlightSelection(aRange, *this); MOZ_KnownLive(registry)->MaybeAddRangeToHighlightSelection(aRange, *this);
if (aRv.Failed()) { if (aRv.Failed()) {
return; return this;
} }
} }
return this;
} }
void Highlight::Clear(ErrorResult& aRv) { void Highlight::Clear(ErrorResult& aRv) {

View File

@@ -144,7 +144,8 @@ class Highlight final : public nsISupports, public nsWrapperCache {
* *
* Also notifies all `HighlightRegistry` instances. * Also notifies all `HighlightRegistry` instances.
*/ */
MOZ_CAN_RUN_SCRIPT void Add(AbstractRange& aRange, ErrorResult& aRv); MOZ_CAN_RUN_SCRIPT Highlight* Add(AbstractRange& aRange,
ErrorResult& aRv);
/** /**
* @brief Removes all ranges from this highlight. * @brief Removes all ranges from this highlight.

View File

@@ -146,18 +146,18 @@ void HighlightRegistry::AddHighlightSelectionsToFrameSelection() {
} }
} }
void HighlightRegistry::Set(const nsAString& aKey, Highlight& aValue, HighlightRegistry* HighlightRegistry::Set(const nsAString& aKey,
ErrorResult& aRv) { Highlight& aValue, ErrorResult& aRv) {
// manually check if the highlight `aKey` is already registered to be able to // manually check if the highlight `aKey` is already registered to be able to
// provide a fast path later that avoids calling `std::find_if()`. // provide a fast path later that avoids calling `std::find_if()`.
const bool highlightAlreadyPresent = const bool highlightAlreadyPresent =
HighlightRegistry_Binding::MaplikeHelpers::Has(this, aKey, aRv); HighlightRegistry_Binding::MaplikeHelpers::Has(this, aKey, aRv);
if (aRv.Failed()) { if (aRv.Failed()) {
return; return this;
} }
HighlightRegistry_Binding::MaplikeHelpers::Set(this, aKey, aValue, aRv); HighlightRegistry_Binding::MaplikeHelpers::Set(this, aKey, aValue, aRv);
if (aRv.Failed()) { if (aRv.Failed()) {
return; return this;
} }
RefPtr<nsFrameSelection> frameSelection = GetFrameSelection(); RefPtr<nsFrameSelection> frameSelection = GetFrameSelection();
RefPtr<nsAtom> highlightNameAtom = NS_AtomizeMainThread(aKey); RefPtr<nsAtom> highlightNameAtom = NS_AtomizeMainThread(aKey);
@@ -184,6 +184,7 @@ void HighlightRegistry::Set(const nsAString& aKey, Highlight& aValue,
if (frameSelection) { if (frameSelection) {
frameSelection->AddHighlightSelection(highlightNameAtom, aValue); frameSelection->AddHighlightSelection(highlightNameAtom, aValue);
} }
return this;
} }
void HighlightRegistry::Clear(ErrorResult& aRv) { void HighlightRegistry::Clear(ErrorResult& aRv) {

View File

@@ -109,8 +109,9 @@ class HighlightRegistry final : public nsISupports, public nsWrapperCache {
* *
* If a `FrameSelection` is present, a highlight selection is created. * If a `FrameSelection` is present, a highlight selection is created.
*/ */
MOZ_CAN_RUN_SCRIPT void Set(const nsAString& aKey, Highlight& aValue, MOZ_CAN_RUN_SCRIPT HighlightRegistry* Set(const nsAString& aKey,
ErrorResult& aRv); Highlight& aValue,
ErrorResult& aRv);
/** /**
* @brief Removes all highlights from this registry. * @brief Removes all highlights from this registry.

View File

@@ -41,7 +41,7 @@ partial interface Highlight {
// Iterating a setlike is not possible from C++ yet. // Iterating a setlike is not possible from C++ yet.
// Therefore a separate data structure must be held and kept in sync. // Therefore a separate data structure must be held and kept in sync.
[Throws] [Throws]
undefined add(AbstractRange range); Highlight add(AbstractRange range);
[Throws] [Throws]
undefined clear(); undefined clear();
[Throws] [Throws]
@@ -63,7 +63,7 @@ partial interface HighlightRegistry {
// Iterating a maplike is not possible from C++ yet. // Iterating a maplike is not possible from C++ yet.
// Therefore, a separate data structure must be held and kept in sync. // Therefore, a separate data structure must be held and kept in sync.
[Throws] [Throws]
undefined set(DOMString key, Highlight value); HighlightRegistry set(DOMString key, Highlight value);
[Throws] [Throws]
undefined clear(); undefined clear();
[Throws] [Throws]

View File

@@ -61,6 +61,15 @@
assert_equals(customHighlight.size, 2, 'Highlight.size is 2 after only adding two different ranges'); assert_equals(customHighlight.size, 2, 'Highlight.size is 2 after only adding two different ranges');
}, 'Highlight add and has methods work as expected' + rangesCombinationDescription); }, 'Highlight add and has methods work as expected' + rangesCombinationDescription);
test(() => {
let customHighlight = new Highlight();
customHighlight.add(rangeCollections[i][0]).add(rangeCollections[i][1]);
assert_true(customHighlight.has(rangeCollections[i][0]), 'Highlight.has returns true when it contains the first range added');
assert_true(customHighlight.has(rangeCollections[i][1]), 'Highlight.has returns true when it contains the second range added');
assert_false(customHighlight.has(rangeCollections[i][2]), 'Highlight.has returns false when it doesn\'t contain the range which is passed as the argument');
assert_equals(customHighlight.size, 2, 'Highlight.size is 2 after adding two different ranges by chaining the add method');
}, "Highlight add method is chainable" + rangesCombinationDescription);
test(() => { test(() => {
let customHighlight = new Highlight(rangeCollections[i][0], rangeCollections[i][1]); let customHighlight = new Highlight(rangeCollections[i][0], rangeCollections[i][1]);
assert_false(customHighlight.delete(rangeCollections[i][2]), 'Highlight.delete returns false when trying to delete a range that is not in the highlight'); assert_false(customHighlight.delete(rangeCollections[i][2]), 'Highlight.delete returns false when trying to delete a range that is not in the highlight');

View File

@@ -70,6 +70,9 @@
assert_equals(CSS.highlights.size, 0, 'HighlightRegistry.clear empties the map.'); assert_equals(CSS.highlights.size, 0, 'HighlightRegistry.clear empties the map.');
CSS.highlights.clear(); CSS.highlights.clear();
assert_equals(CSS.highlights.size, 0, 'HighlightRegistry.clear called with an empty registry keeps it empty.'); assert_equals(CSS.highlights.size, 0, 'HighlightRegistry.clear called with an empty registry keeps it empty.');
CSS.highlights.set(name1, h1).set(name2, h2);
assert_equals(CSS.highlights.size, 2, 'HighlightRegistry.size is 2 after inserting two different Highlights by chaining the set method.');
}, 'HighlightRegistry has a maplike interface.'); }, 'HighlightRegistry has a maplike interface.');
</script> </script>