Implement Named constructors and the Image constructor for HTMLImageElement

[klusark]

I'm not sure if I like how I mostly just duplicated the code in CodegenRust.py, so that might need to be refactored.

Instead of just calling it Image, we might want to call it ConstructorImage, to make it clear that it's a constructor. Anyone have an opinion on that?

There seems to be a bug in the HTMLImageElement getter/setter as the value is 0 regardless of what I do. This seems to be unrelated to my commits, so I'll investigate that separately.

[highfive]

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

[Manishearth]

The width IDL attribute is a reflector, and there are additional constraints on the width content attr, which is what's happening here. The internal implementation works with the bounding box.

https://html.spec.whatwg.org/multipage/embedded-content.html#attr-dim-width

There still may be a mistake here though, we should look closely.

[Manishearth]

Scratch that, our implementation is correct.

https://html.spec.whatwg.org/multipage/embedded-content.html#dom-img-width

That's what I get for clicking the wrong link and not noticing that img wasn't mentioned

[Manishearth]

r? @Ms2ger

[KiChjang]

Remember to close #2540 along the way

[hoppipolla-critic-bot]

Critic review: https://critic.hoppipolla.co.uk/r/5049

This is an external review system which you may optionally use for the code review of your pull request.

In order to help critic track your changes, please do not make in-place history rewrites (e.g. via git rebase -i or git commit --amend) when updating this pull request.

[Ms2ger]

Reviewed on critic

[klusark]

I'm not sure about the test I added. Is it comprehensive enough? I'm not really sure what else I could add to it.

[bors-servo]

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

[frewsxcv]

It looks like one of the lines is beyond 120 characters

https://travis-ci.org/servo/servo/builds/63910947

[Ms2ger]

Sorry I've been dragging my feet. I've added more comments on critic.

[bors-servo]

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

[Ms2ger]

Please rebase on master.

[klusark]

I rebased it, but I haven't addressed the review comments yet.

[klusark]

That should resolve all the comments now. If it's good now I'll squash it into two commits. One for NamedConstructor support and one for adding the named constructor to Image

[Ms2ger]

+S-needs-code-changes
Thanks! I'm afraid I have a few more comments still, but it's looking pretty good!

---

Reviewed 1 of 2 files at r2, 3 of 5 files at r7, 1 of 2 files at r8, 1 of 1 files at r9.
Review status: 6 of 7 files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

^{components/script/dom/bindings/utils.rs, line 203 [r9] (raw file):}
I don't think we need the Option<> around the entries here.

---

^{components/script/dom/bindings/utils.rs, line 218 [r9] (raw file):}
This code is here twice now

---

^{components/script/dom/bindings/utils.rs, line 228 [r9] (raw file):}
Also translate

      if (!JS_DefineProperty(cx, namedConstructor, "prototype",
                             proto,
                             JSPROP_PERMANENT | JSPROP_READONLY,
                             JS_STUBGETTER, JS_STUBSETTER)) {
        return nullptr;
      }

---

^{components/script/dom/bindings/utils.rs, line 274 [r9] (raw file):}
Please move this back to its original place

---

^{components/script/dom/htmlimageelement.rs, line 13 [r9] (raw file):}
Nit: no {}s

---

^{tests/wpt/metadata/html/dom/interfaces.html.ini, line 9024 [r9] (raw file):}
Why does this fail?

---

Comments from the review on Reviewable.io

[klusark]

Review status: 6 of 7 files reviewed at latest revision, 5 unresolved discussions, all commit checks successful.

---

^{tests/wpt/metadata/html/dom/interfaces.html.ini, line 9024 [r9] (raw file):}
I'm trying to verify if this still fails, but now around half the web platform tests are failing for me, even on master...

---

Comments from the review on Reviewable.io

[Ms2ger]

Reviewed 4 of 4 files at r10, 1 of 1 files at r11.
Review status: all files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

^{components/script/dom/bindings/codegen/CodegenRust.py, line 4565 [r11] (raw file):}
I don't think this override is necessary

---

Comments from the review on Reviewable.io

[Ms2ger]

-S-awaiting-review +S-needs-code-changes
Nearly there! Thank you so much.

---

Review status: all files reviewed at latest revision, 6 unresolved discussions, all commit checks successful.

---

Comments from the review on Reviewable.io

[Ms2ger]

-S-awaiting-review +S-needs-squash

\o/

---

Reviewed 1 of 1 files at r12.
Review status: all files reviewed at latest revision, all discussions resolved, all commit checks successful.

---

Comments from the review on Reviewable.io

[Ms2ger]

@bors-servo: r+

[bors-servo]

📌 Commit d335791 has been approved by Ms2ger

[bors-servo]

⌛ Testing commit d335791 with merge 57cc84b...

[bors-servo]

☀️ Test successful - android, gonk, linux1, linux2, linux3, mac1, mac2, mac3