1587716 - `RangeBoundary` constructor receives negative offsets, but converts them to an unsigned integer

Status: RESOLVED FIXED

(Core :: DOM: Selection, defect, P2)

Description

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

STR:

1. Add assertion:

   RangeBoundaryBase(nsINode* aContainer, int32_t aOffset)
-      : mParent(aContainer), mRef(nullptr), mOffset(mozilla::Some(aOffset)) {
+      : mParent(aContainer), mRef(nullptr), mOffset(mozilla::Some<uint32_t>(aOffset)) {
+    MOZ_ASSERT(aOffset >= 0)

2. Run ./testing/web-platform/tests/selection/collapse-00.html

Expected: assertion not triggered.

Actual: assertion triggered.

Comment 1

[Edgar Chen [:edgar] (OOO untils 09/01, slow response)]

(P2 given that we probably will start working on this in following months, please free feel to change the priority if I misunderstand anything)

Comment 2

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

One problem here is: nsRange::ComparePoint has a uint32_t argument, which is required by https://dom.spec.whatwg.org/#interface-range. But it internally calls RangeBoundaryBase::RangeBoundaryBase, which takes a int32_t.
Reproducible with dom/base/test/test_bug737565.html.

Another violation of the assertion is triggered after running and successfully finishing editor/libeditor/tests/test_bug1270235.html.

Comment 3

[Mirko Brodesser (:mbrodesser) -- not reading bugmails currently]

Attachment: Bug 1587716: change `RangeBoundaryBase`'s offset argument to `uint32_t`. r=smaug

uint32_t, because nsRange::ComparePoints requires it -- by webidl
interface -- to be unsigned long.

Moreover it makes RangeBoundaryBase's interface cleaner, because it
already exposes the offset as a uint32_t.

Comment 4

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/8e233a18ab9a
change `RangeBoundaryBase`'s offset argument to `uint32_t`. r=smaug

Comment 5

[Andreea Pavel [:apavel]]

https://hg.mozilla.org/mozilla-central/rev/8e233a18ab9a