Implement Range.getClientRects and Range.getBoudingClientRect (Work in progress)

[jaysonsantos]

Hi guys, I did this pull request so you can check if I am heading on the right track.
This should fix #8982 when done.

---

This change is

[highfive]

Thanks for the pull request, and welcome! The Servo team is excited to review your changes, and you should hear from @Manishearth (or someone else) soon.

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/script/dom/webidls/Range.webidl, components/script/dom/range.rs

[Manishearth]

r? @KiChjang

might not get time for this till next week

[frewsxcv]

components/script/dom/range.rs, line 910 [r13] (raw file):
personally, I'd do:

let node = match node {
    Some(node) => node,
    None => return;
};

...

So it's not one giant indented block.

---

Comments from Reviewable

[KiChjang]

Reviewed 1 of 2 files at r1, 1 of 2 files at r8, 1 of 2 files at r11, 1 of 1 files at r12, 1 of 1 files at r14.
Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

components/script/dom/range.rs, line 30 [r14] (raw file):
nit: Remove the braces.

---

components/script/dom/range.rs, line 909 [r14] (raw file):
Is the third &Range argument necessary? You can simply use self.

---

components/script/dom/range.rs, line 920 [r14] (raw file):
This is very imperative-styled, we should instead make a new method in DOMRectList that returns its underlying Vec<JS<DOMRect>> and use an iterator over this vec instead.

---

components/script/dom/range.rs, line 929 [r14] (raw file):
Are these 3 lines necessary?

---

components/script/dom/range.rs, line 978 [r14] (raw file):
Step annotations here would be nice.

---

components/script/dom/range.rs, line 979 [r14] (raw file):

Otherwise, return a static DOMRect object describing the smallest rectangle that includes the first rectangle in list and all of the remaining rectangles of which the height or width is not zero.

This does not look like it is following the spec.

---

tests/wpt/web-platform-tests/dom/ranges/Range-getBoundingClientRect.html, line 17 [r14] (raw file):
Are these random values or magic values?

---

tests/wpt/web-platform-tests/dom/ranges/Range-getClientRects.html, line 20 [r14] (raw file):
Same here, random or magic?

---

Comments from Reviewable

[jaysonsantos]

Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

components/script/dom/range.rs, line 909 [r14] (raw file):
It will say that I need to move self to be able to access it.

---

components/script/dom/range.rs, line 929 [r14] (raw file):
The same, the compiler will say that I need to move to use it here.

---

components/script/dom/range.rs, line 979 [r14] (raw file):
No, this is the latest point that I will look into

---

tests/wpt/web-platform-tests/dom/ranges/Range-getBoundingClientRect.html, line 17 [r14] (raw file):
I looked on the return values of chrome. Do you have any suggestions to make it?

---

tests/wpt/web-platform-tests/dom/ranges/Range-getClientRects.html, line 20 [r14] (raw file):
The same as the other

---

Comments from Reviewable

[KiChjang]

Review status: all files reviewed at latest revision, 9 unresolved discussions, some commit checks failed.

---

components/script/dom/range.rs, line 909 [r14] (raw file):
Then i don't see the reason why this is an inner function. You can move this outside and take a &self parameter.

---

Comments from Reviewable

[highfive]

New code was committed to pull request.

[jdm]

Looks like that rebase didn't work quite right...

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[bors-servo]

☔ The latest upstream changes (presumably #11496) made this pull request unmergeable. Please resolve the merge conflicts.

[KiChjang]

@jaysonsantos Is this still being worked on?

[jaysonsantos]

Hey @KiChjang sorry for the late response. I wasn't able to understand the architecture of servo and the specs and I didn't have time these days. I think it is better to let someone that already know it and I will get an easier issue later.

[KiChjang]

Okay, no problem!