fix getElementsByTagName()

[kevgs]

#11596

@nox

This is a first iteration. I need some review.

---

• ./mach build -d does not report any errors
• ./mach test-tidy does not report any errors
• These changes fix #__ (github issue number if applicable).

• There are tests for these changes OR
• These changes do not require tests because _____

---

This change is

[highfive]

Heads up! This PR modifies the following files:

• @KiChjang: components/script/dom/document.rs, components/script/dom/htmlcollection.rs, components/script/dom/webidls/Document.webidl, components/script/dom/element.rs

[highfive]

New code was committed to pull request.

[highfive]

New code was committed to pull request.

[KiChjang]

r? @nox

[nox]

-S-awaiting-review +S-awaiting-answer +S-needs-code-changes

---

Reviewed 10 of 10 files at r1, 1 of 1 files at r2, 1 of 1 files at r3.
Review status: all files reviewed at latest revision, 4 unresolved discussions.

---

a discussion (no related file):
I think we should have new tests that look for test:AÇ in a HTML document, no?

---

a discussion (no related file):
I don't think this patch account for:

When invoked with the same argument, and as long as root’s node document’s type has not changed, the same HTMLCollection object may be returned as returned by an earlier call.

Can you confirm?

---

components/script/dom/document.rs, line 1992 [r1] (raw file):

    // https://dom.spec.whatwg.org/#dom-document-getelementsbytagname
    fn GetElementsByTagName(&self, qualified_name: DOMString) -> Root<HTMLCollection> {
        let qualified_name_atom = Atom::from(&*qualified_name);

Nit: please just do this while you are at it:

let qualified_name = Atom::from(qualified_name);

We don't try to avoid shadowing in such cases. Converting directly from DOMString also means that we don't need to allocate a new String if no Atom with that name existed previously.

---

components/script/dom/htmlcollection.rs, line 117 [r1] (raw file):

    }

    pub fn match_elements_by_qualified_name(window: &Window, root: &Node, mut qualified_name: DOMString)

You should just pass an Atom here, given we already atomify the qualified name in caller.

---

Comments from Reviewable

[jdm]

@kevgs Are you still planning to finish this up?

[kevgs]

I would to but I can't because I'm mostly afk now. I'm planning to continue to work on Servo in 2-3 weeks. Sorry for that.

05.07.2016, 00:17, "Josh Matthews" notifications@github.com:

@kevgs Are you still planning to finish this up?

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Eugene

[KiChjang]

Let us know if you start working on this again. I'll close this PR in the meantime. Thanks for your work!