Add picture element #19431

Rakhisharma · 2017-11-30T09:55:46Z

./mach build -d does not report any errors
./mach test-tidy does not report any errors
These changes are part of #11416 (github issue number if applicable).

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

This change is

highfive · 2017-11-30T09:55:53Z

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

highfive · 2017-11-30T09:55:54Z

Heads up! This PR modifies the following files:

@asajeffrey: components/script/dom/webidls/HTMLPictureElement.webidl, components/script/dom/htmlpictureelement.rs
@fitzgen: components/script/dom/webidls/HTMLPictureElement.webidl, components/script/dom/htmlpictureelement.rs
@KiChjang: components/script/dom/webidls/HTMLPictureElement.webidl, components/script/dom/htmlpictureelement.rs

highfive · 2017-11-30T09:55:55Z

Warning

These commits modify script code, but no tests are modified. Please consider adding a test!

Rakhisharma · 2017-11-30T09:57:53Z

@jdm are we good to go?

KiChjang

I believe we would also need to make changes to https://github.com/servo/servo/blob/master/components/script/dom/mod.rs, adding htmlpictureelement to the list, and this list as well.

KiChjang · 2017-11-30T11:13:18Z

components/script/dom/webidls/HTMLPictureElement.webidl

+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+// https://html.spec.whatwg.org/multipage/#htmlimageelement


Shouldn't this link to https://html.spec.whatwg.org/multipage/#htmlpictureelement?

ah, yes this should be https://html.spec.whatwg.org/multipage/#htmlpictureelement. I will do the required changes and update. Thanks.

Rakhisharma · 2017-11-30T13:24:12Z

@KiChjang I am trying to add htmlpictureelement to this list but I a getting build error. Please see the error here. How can I resolve this? I am not able to find any reference.

jdm · 2017-12-01T05:59:40Z

@Rakhisharma You need to add it to https://github.com/servo/html5ever/blob/master/markup5ever/local_names.txt unfortunately.

Rakhisharma · 2017-12-08T07:28:19Z

@jdm I added picture to servo/html5ever. In spite of adding that I am facing few build errors here. Also in Janitor, I have the copy of servo/servo repo, not servo/html5ever- how this works(probably I want to ask how they are communicating with each other)? because I can see the same picture error in my janitor build(I was facing same error before adding picture to servo/html5ever)- https://gist.github.com/Rakhisharma/2b31baece5bcb8d2fb92a520faab6c74.

wafflespeanut · 2017-12-08T12:32:06Z

The Cargo.toml wasn't updated. I talked to her in IRC.

jdm · 2017-12-08T14:43:42Z

./mach test-tidy says:

./components/script/dom/htmlpictureelement.rs:13: trailing whitespace
./components/script/dom/htmlpictureelement.rs:13: found an empty line following a {
./components/script/dom/webidls/HTMLPictureElement.webidl:4: trailing whitespace

From building:

error[E0432]: unresolved import `dom::htmlpictureelement::HTMLPictureElement`
  --> /home/travis/build/servo/servo/components/script/dom/create.rs:57:5
   |
57 | use dom::htmlpictureelement::HTMLPictureElement;
   |     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `HTMLPictureElement` in `dom::htmlpictureelement`
error[E0432]: unresolved import `dom::htmlpictureelement::HTMLPictureElement`
   --> /home/travis/build/sevo/servo/target/debug/build/script-520c6c9af5ee635e/out/InterfaceTypes.rs:131:9
    |
131 | pub use dom::htmlpictureelement::HTMLPictureElement;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `HTMLPictureElement` in `dom::htmlpictureelement`
error[E0432]: unresolved import `dom::htmlpictureelement::HTMLPictureElement`
   --> /home/travis/build/sevo/servo/target/debug/build/script-520c6c9af5ee635e/out/InterfaceTypes.rs:131:9
    |
131 | pub use dom::htmlpictureelement::HTMLPictureElement;
    |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ no `HTMLPictureElement` in `dom::htmlpictureelement`
error: cannot find attribute macro `dom_struct` in this scope
 --> /home/travis/build/servo/servo/components/script/dom/htmlpictureelement.rs:7:3
  |
7 | #[dom_struct]
  |   ^^^^^^^^^^
error[E0412]: cannot find type `HTMLPictureElement` in this scope
  --> /home/travis/build/servo/servo/components/script/dom/htmlpictureelement.rs:12:6
   |
12 | impl HTMLPictureElement {
   |      ^^^^^^^^^^^^^^^^^^ not found in this scope
help: you can try using the variant's enum
   |
12 | impl dom::bindings::codegen::InheritTypes::HTMLElementTypeId {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can try using the variant's enum
   |
12 | impl dom::bindings::codegen::PrototypeList::ID {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
help: you can try using the variant's enum
   |
12 | impl dom::bindings::codegen::PrototypeList::Constructor {
   |      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
error: aborting due to 5 previous errors

jdm · 2017-12-08T14:47:03Z

components/script/dom/htmlpictureelement.rs

+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
+
+use dom::htmlelement::HTMLElement;


You need use dom_struct::dom_struct; as well.

Rakhisharma · 2018-01-04T14:02:19Z

r? @jdm

jdm

We've lost the changes to

servo/components/script/dom/create.rs

Lines 215 to 358 in cc8f930

    
           match name.local { 
        
               local_name!("a")          => make!(HTMLAnchorElement), 
        
               local_name!("abbr")       => make!(HTMLElement), 
        
               local_name!("acronym")    => make!(HTMLElement), 
        
               local_name!("address")    => make!(HTMLElement), 
        
               local_name!("area")       => make!(HTMLAreaElement), 
        
               local_name!("article")    => make!(HTMLElement), 
        
               local_name!("aside")      => make!(HTMLElement), 
        
               local_name!("audio")      => make!(HTMLAudioElement), 
        
               local_name!("b")          => make!(HTMLElement), 
        
               local_name!("base")       => make!(HTMLBaseElement), 
        
               local_name!("bdi")        => make!(HTMLElement), 
        
               local_name!("bdo")        => make!(HTMLElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:bgsound 
        
               local_name!("bgsound")    => make!(HTMLUnknownElement), 
        
               local_name!("big")        => make!(HTMLElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:blink 
        
               local_name!("blink")      => make!(HTMLUnknownElement), 
        
               // https://html.spec.whatwg.org/multipage/#the-blockquote-element 
        
               local_name!("blockquote") => make!(HTMLQuoteElement), 
        
               local_name!("body")       => make!(HTMLBodyElement), 
        
               local_name!("br")         => make!(HTMLBRElement), 
        
               local_name!("button")     => make!(HTMLButtonElement), 
        
               local_name!("canvas")     => make!(HTMLCanvasElement), 
        
               local_name!("caption")    => make!(HTMLTableCaptionElement), 
        
               local_name!("center")     => make!(HTMLElement), 
        
               local_name!("cite")       => make!(HTMLElement), 
        
               local_name!("code")       => make!(HTMLElement), 
        
               local_name!("col")        => make!(HTMLTableColElement), 
        
               local_name!("colgroup")   => make!(HTMLTableColElement), 
        
               local_name!("data")       => make!(HTMLDataElement), 
        
               local_name!("datalist")   => make!(HTMLDataListElement), 
        
               local_name!("dd")         => make!(HTMLElement), 
        
               local_name!("del")        => make!(HTMLModElement), 
        
               local_name!("details")    => make!(HTMLDetailsElement), 
        
               local_name!("dfn")        => make!(HTMLElement), 
        
               local_name!("dialog")     => make!(HTMLDialogElement), 
        
               local_name!("dir")        => make!(HTMLDirectoryElement), 
        
               local_name!("div")        => make!(HTMLDivElement), 
        
               local_name!("dl")         => make!(HTMLDListElement), 
        
               local_name!("dt")         => make!(HTMLElement), 
        
               local_name!("em")         => make!(HTMLElement), 
        
               local_name!("embed")      => make!(HTMLEmbedElement), 
        
               local_name!("fieldset")   => make!(HTMLFieldSetElement), 
        
               local_name!("figcaption") => make!(HTMLElement), 
        
               local_name!("figure")     => make!(HTMLElement), 
        
               local_name!("font")       => make!(HTMLFontElement), 
        
               local_name!("footer")     => make!(HTMLElement), 
        
               local_name!("form")       => make!(HTMLFormElement), 
        
               local_name!("frame")      => make!(HTMLFrameElement), 
        
               local_name!("frameset")   => make!(HTMLFrameSetElement), 
        
               local_name!("h1")         => make!(HTMLHeadingElement, HeadingLevel::Heading1), 
        
               local_name!("h2")         => make!(HTMLHeadingElement, HeadingLevel::Heading2), 
        
               local_name!("h3")         => make!(HTMLHeadingElement, HeadingLevel::Heading3), 
        
               local_name!("h4")         => make!(HTMLHeadingElement, HeadingLevel::Heading4), 
        
               local_name!("h5")         => make!(HTMLHeadingElement, HeadingLevel::Heading5), 
        
               local_name!("h6")         => make!(HTMLHeadingElement, HeadingLevel::Heading6), 
        
               local_name!("head")       => make!(HTMLHeadElement), 
        
               local_name!("header")     => make!(HTMLElement), 
        
               local_name!("hgroup")     => make!(HTMLElement), 
        
               local_name!("hr")         => make!(HTMLHRElement), 
        
               local_name!("html")       => make!(HTMLHtmlElement), 
        
               local_name!("i")          => make!(HTMLElement), 
        
               local_name!("iframe")     => make!(HTMLIFrameElement), 
        
               local_name!("img")        => make!(HTMLImageElement), 
        
               local_name!("input")      => make!(HTMLInputElement), 
        
               local_name!("ins")        => make!(HTMLModElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:isindex-2 
        
               local_name!("isindex")    => make!(HTMLUnknownElement), 
        
               local_name!("kbd")        => make!(HTMLElement), 
        
               local_name!("label")      => make!(HTMLLabelElement), 
        
               local_name!("legend")     => make!(HTMLLegendElement), 
        
               local_name!("li")         => make!(HTMLLIElement), 
        
               local_name!("link")       => make!(HTMLLinkElement, creator), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:listing 
        
               local_name!("listing")    => make!(HTMLPreElement), 
        
               local_name!("main")       => make!(HTMLElement), 
        
               local_name!("map")        => make!(HTMLMapElement), 
        
               local_name!("mark")       => make!(HTMLElement), 
        
               local_name!("marquee")    => make!(HTMLElement), 
        
               local_name!("meta")       => make!(HTMLMetaElement), 
        
               local_name!("meter")      => make!(HTMLMeterElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:multicol 
        
               local_name!("multicol")   => make!(HTMLUnknownElement), 
        
               local_name!("nav")        => make!(HTMLElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:nextid 
        
               local_name!("nextid")     => make!(HTMLUnknownElement), 
        
               local_name!("nobr")       => make!(HTMLElement), 
        
               local_name!("noframes")   => make!(HTMLElement), 
        
               local_name!("noscript")   => make!(HTMLElement), 
        
               local_name!("object")     => make!(HTMLObjectElement), 
        
               local_name!("ol")         => make!(HTMLOListElement), 
        
               local_name!("optgroup")   => make!(HTMLOptGroupElement), 
        
               local_name!("option")     => make!(HTMLOptionElement), 
        
               local_name!("output")     => make!(HTMLOutputElement), 
        
               local_name!("p")          => make!(HTMLParagraphElement), 
        
               local_name!("param")      => make!(HTMLParamElement), 
        
               local_name!("plaintext")  => make!(HTMLPreElement), 
        
               local_name!("pre")        => make!(HTMLPreElement), 
        
               local_name!("progress")   => make!(HTMLProgressElement), 
        
               local_name!("q")          => make!(HTMLQuoteElement), 
        
               local_name!("rp")         => make!(HTMLElement), 
        
               local_name!("rt")         => make!(HTMLElement), 
        
               local_name!("ruby")       => make!(HTMLElement), 
        
               local_name!("s")          => make!(HTMLElement), 
        
               local_name!("samp")       => make!(HTMLElement), 
        
               local_name!("script")     => make!(HTMLScriptElement, creator), 
        
               local_name!("section")    => make!(HTMLElement), 
        
               local_name!("select")     => make!(HTMLSelectElement), 
        
               local_name!("small")      => make!(HTMLElement), 
        
               local_name!("source")     => make!(HTMLSourceElement), 
        
               // https://html.spec.whatwg.org/multipage/#other-elements,-attributes-and-apis:spacer 
        
               local_name!("spacer")     => make!(HTMLUnknownElement), 
        
               local_name!("span")       => make!(HTMLSpanElement), 
        
               local_name!("strike")     => make!(HTMLElement), 
        
               local_name!("strong")     => make!(HTMLElement), 
        
               local_name!("style")      => make!(HTMLStyleElement, creator), 
        
               local_name!("sub")        => make!(HTMLElement), 
        
               local_name!("summary")    => make!(HTMLElement), 
        
               local_name!("sup")        => make!(HTMLElement), 
        
               local_name!("table")      => make!(HTMLTableElement), 
        
               local_name!("tbody")      => make!(HTMLTableSectionElement), 
        
               local_name!("td")         => make!(HTMLTableDataCellElement), 
        
               local_name!("template")   => make!(HTMLTemplateElement), 
        
               local_name!("textarea")   => make!(HTMLTextAreaElement), 
        
               // https://html.spec.whatwg.org/multipage/#the-tfoot-element:concept-element-dom 
        
               local_name!("tfoot")      => make!(HTMLTableSectionElement), 
        
               local_name!("th")         => make!(HTMLTableHeaderCellElement), 
        
               // https://html.spec.whatwg.org/multipage/#the-thead-element:concept-element-dom 
        
               local_name!("thead")      => make!(HTMLTableSectionElement), 
        
               local_name!("time")       => make!(HTMLTimeElement), 
        
               local_name!("title")      => make!(HTMLTitleElement), 
        
               local_name!("tr")         => make!(HTMLTableRowElement), 
        
               local_name!("tt")         => make!(HTMLElement), 
        
               local_name!("track")      => make!(HTMLTrackElement), 
        
               local_name!("u")          => make!(HTMLElement), 
        
               local_name!("ul")         => make!(HTMLUListElement), 
        
               local_name!("var")        => make!(HTMLElement), 
        
               local_name!("video")      => make!(HTMLVideoElement), 
        
               local_name!("wbr")        => make!(HTMLElement), 
        
               local_name!("xmp")        => make!(HTMLPreElement), 
        
               _ if is_valid_custom_element_name(&*name.local) => make!(HTMLElement), 
        
               _                         => make!(HTMLUnknownElement), 
        
           }

that were previously requested.

jdm · 2018-01-04T18:03:30Z

@bors-servo: try

bors-servo · 2018-01-04T18:03:35Z

⌛ Trying commit 72be822 with merge b652d6c...


        Auto merge of #19431 - Rakhisharma:picture_element, r=<try>

Add picture element  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes are part of #11416 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19431)

jdm · 2018-01-30T01:06:26Z

@bors-servo r+

bors-servo · 2018-01-30T01:06:26Z

📌 Commit 590ffd3 has been approved by jdm

bors-servo · 2018-01-30T01:06:31Z

⌛ Testing commit 590ffd3 with merge a53e22f...


        Auto merge of #19431 - Rakhisharma:picture_element, r=jdm

Add picture element  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes are part of #11416 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19431)

bors-servo · 2018-01-30T01:23:27Z

💔 Test failed - mac-dev-unit


        Implement HTMLPictureElement interface.

jdm · 2018-01-30T01:29:53Z

@bors-servo r+

bors-servo · 2018-01-30T01:29:53Z

📌 Commit d47ce85 has been approved by jdm

bors-servo · 2018-01-30T01:29:57Z

⌛ Testing commit d47ce85 with merge e49e861...


        Auto merge of #19431 - Rakhisharma:picture_element, r=jdm

Add picture element  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes are part of #11416 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19431)

bors-servo · 2018-01-30T02:00:28Z

💔 Test failed - linux-rel-css

jdm · 2018-01-30T13:07:39Z

@bors-servo retry

bors-servo · 2018-01-30T13:07:45Z

⌛ Testing commit d47ce85 with merge 76b4e5c...


        Auto merge of #19431 - Rakhisharma:picture_element, r=jdm

Add picture element  ---  - [x] `./mach build -d` does not report any errors - [x] `./mach test-tidy` does not report any errors - [ ] These changes are part of #11416 (github issue number if applicable).  - [ ] There are tests for these changes OR - [ ] These changes do not require tests because _____    --- This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19431)

bors-servo · 2018-01-30T13:56:42Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel-css, linux-rel-wpt, mac-dev-unit, mac-rel-css1, mac-rel-css2, mac-rel-wpt1, mac-rel-wpt2, mac-rel-wpt3, mac-rel-wpt4, windows-msvc-dev
Approved by: jdm
Pushing 76b4e5c to master...

highfive assigned glennw Nov 30, 2017

highfive added the S-awaiting-review label Nov 30, 2017

KiChjang requested changes Nov 30, 2017

View changes

Rakhisharma mentioned this pull request Dec 7, 2017

Add picture local name for picture element servo/html5ever#331

Merged

wafflespeanut added S-needs-code-changes and removed S-awaiting-review labels Dec 8, 2017

jdm requested changes Dec 8, 2017

View changes

Rakhisharma force-pushed the Rakhisharma:picture_element branch from 05cd853 to 97f9241 Jan 4, 2018

highfive added S-awaiting-review and removed S-needs-code-changes labels Jan 4, 2018

highfive assigned jdm and unassigned glennw Jan 4, 2018

jdm requested changes Jan 4, 2018

View changes

jdm added S-needs-code-changes and removed S-awaiting-review labels Jan 4, 2018

highfive added S-awaiting-review and removed S-needs-code-changes labels Jan 4, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 30, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 30, 2018

Implement HTMLPictureElement interface.

Loading status checks…

d47ce85

jdm force-pushed the Rakhisharma:picture_element branch from 590ffd3 to d47ce85 Jan 30, 2018

highfive added S-awaiting-review and removed S-tests-failed labels Jan 30, 2018

highfive added S-awaiting-merge and removed S-awaiting-review labels Jan 30, 2018

highfive added S-tests-failed and removed S-awaiting-merge labels Jan 30, 2018

highfive added S-awaiting-merge and removed S-tests-failed labels Jan 30, 2018

bors-servo merged commit d47ce85 into servo:master Jan 30, 2018
3 checks passed

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details

continuous-integration/travis-ci/pr The Travis CI build passed
Details

homu Test successful
Details

highfive removed the S-awaiting-merge label Jan 30, 2018

servo / servo

Add picture element #19431

Add picture element #19431

Rakhisharma commented Nov 30, 2017 •

edited

highfive commented Nov 30, 2017

highfive commented Nov 30, 2017

highfive commented Nov 30, 2017

Rakhisharma commented Nov 30, 2017

KiChjang left a comment

This comment has been minimized.

This comment has been minimized.

Rakhisharma commented Nov 30, 2017

jdm commented Dec 1, 2017

Rakhisharma commented Dec 8, 2017

wafflespeanut commented Dec 8, 2017

jdm commented Dec 8, 2017

This comment has been minimized.

Rakhisharma commented Jan 4, 2018

jdm left a comment

jdm commented Jan 4, 2018

bors-servo commented Jan 4, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

servo / servo

Join GitHub today

Add picture element #19431

Add picture element #19431

Conversation

Rakhisharma commented Nov 30, 2017 • edited

highfive commented Nov 30, 2017

highfive commented Nov 30, 2017

highfive commented Nov 30, 2017

Rakhisharma commented Nov 30, 2017

KiChjang left a comment

This comment has been minimized.

KiChjang Nov 30, 2017

This comment has been minimized.

Rakhisharma Nov 30, 2017

Rakhisharma commented Nov 30, 2017

jdm commented Dec 1, 2017

Rakhisharma commented Dec 8, 2017

wafflespeanut commented Dec 8, 2017

jdm commented Dec 8, 2017

This comment has been minimized.

jdm Dec 8, 2017

Rakhisharma commented Jan 4, 2018

jdm left a comment

jdm commented Jan 4, 2018

bors-servo commented Jan 4, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

jdm commented Jan 30, 2018

bors-servo commented Jan 30, 2018

bors-servo commented Jan 30, 2018

Rakhisharma commented Nov 30, 2017 •

edited