indexed getter of form elements #11697

mrmiywj · 2016-06-10T07:24:05Z

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

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

This change is

highfive · 2016-06-10T07:24:11Z

Heads up! This PR modifies the following files:

@KiChjang: components/script/dom/htmlformelement.rs, components/script/dom/webidls/HTMLFormElement.webidl

Ms2ger · 2016-06-10T09:34:36Z

You still need to fix the comments made on the previous PR.

mrmiywj · 2016-06-10T10:22:48Z

@Ms2ger
Hmm, I think I've fixed the comments made on the previous PR.
This comment is not avoidable for now.

Other comments have been fixed.

Ms2ger · 2016-06-10T11:23:01Z

components/script/dom/htmlformelement.rs

+    // https://html.spec.whatwg.org/multipage/#dom-form-item
+    fn IndexedGetter(&self, index: u32, found: &mut bool) -> Option<Root<Element>> {
+        let elements = self.Elements();
+        (*elements).IndexedGetter(index, found)


Should be no need for the explicit dereference.

Ms2ger · 2016-06-10T11:23:10Z

tests/wpt/web-platform-tests/html/semantics/forms/the-form-element/form-indexed-element.html

+<form id=form>
+<table>
+<tr>
+<td><input type="radio" name="radio1" id="r1" value=1></td>


Why the table?

highfive · 2016-06-10T11:28:38Z

New code was committed to pull request.

mrmiywj · 2016-06-10T11:29:48Z

Sorry for that I didn't notice that it was changed on my other PC. My bad.

KiChjang · 2016-06-11T04:19:59Z

Isn't #11405 (the issue that this PR is trying to fix) supposed to be assigned to @catchmrbharath?

mrmiywj · 2016-06-11T04:25:26Z

Sorry for that I did not notice that. This issue was on my ToDo list and still open, so I just tried to pick it up.

highfive · 2016-06-12T04:20:52Z

New code was committed to pull request.

highfive · 2016-06-12T04:24:02Z

New code was committed to pull request.

KiChjang · 2016-06-13T08:31:12Z

components/script/dom/webidls/HTMLFormElement.webidl

@@ -17,7 +17,7 @@ interface HTMLFormElement : HTMLElement {

  [SameObject] readonly attribute HTMLFormControlsCollection elements;
  readonly attribute unsigned long length;
-  //getter Element (unsigned long index);
+  getter Element? (unsigned long index);


This isn't following the WebIDL definitions for HTMLFormElement.

This cannot be avoided for now.

KiChjang · 2016-06-13T08:32:42Z

tests/wpt/web-platform-tests/html/semantics/forms/the-form-element/form-indexed-element.html

+<tr>
+<td><input type="radio" name="radio1" id="r1" value=1></td>
+<td><input type="radio" name="radio2" id="r2" value=2></td>
+</tr>


Following on to @Ms2ger's comment, why is this still under a <tr> and <td> tag?

highfive · 2016-06-13T09:19:29Z

New code was committed to pull request.

mrmiywj · 2016-06-13T09:19:57Z

@KiChjang Sorry for that I'm not familiar with FE programming.

Ms2ger · 2016-06-13T10:32:02Z

I'm sorry to do this to you, but @KiChjang was wrong about the ? in the WebIDL file. In our implementation, it needs to be there.

highfive · 2016-06-13T11:29:22Z

New code was committed to pull request.

mrmiywj · 2016-06-13T11:30:56Z

Yep, ? is not avoided here since the tests failed. Is there any resources about the webidls generator specification used here?

jdm · 2016-06-13T12:00:42Z

https://developer.mozilla.org/en-US/docs/Mozilla/WebIDL_bindings and http://doc.servo.org/script/dom/bindings/index.html might be what you're looking for, or else http://heycam.github.io/webidl/ is the actual specification for WebIDL files.

highfive · 2016-06-13T12:07:30Z

New code was committed to pull request.

bors-servo · 2016-06-14T09:55:10Z

💔 Test failed - mac-rel-wpt

highfive · 2016-06-14T09:55:14Z

  ▶ Unexpected subtest result in /old-tests/submission/Opera/script_scheduling/031.html:
  └ PASS [expected FAIL]  scheduler: focus and blur events

jdm · 2016-06-14T12:51:35Z

That test uses indexed getters on form elements, so that's a pleasant surprise :)

highfive · 2016-06-14T18:22:45Z

New code was committed to pull request.

jdm · 2016-06-15T00:21:13Z

Please squash the commits :)


        indexed getter of form elements

mrmiywj · 2016-06-15T04:43:16Z

Sorry, I forgot that;)

KiChjang · 2016-06-15T06:10:46Z

@bors-servo r=Ms2ger

bors-servo · 2016-06-15T06:10:47Z

📌 Commit 4bb8843 has been approved by Ms2ger

bors-servo · 2016-06-15T06:10:49Z

⌛ Testing commit 4bb8843 with merge 2086d21...


        Auto merge of #11697 - mrmiywj:form-index-getter, r=Ms2ger

indexed getter of form elements  ---  - [X] `./mach build -d` does not report any errors - [X] `./mach test-tidy` does not report any errors - [X] These changes fix #11405 (github issue number if applicable).  - [X] 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="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11697)

bors-servo · 2016-06-15T06:58:11Z

💔 Test failed - linux-rel

highfive · 2016-06-15T06:58:15Z

  ▶ FAIL [expected PASS] /css-transforms-1_dev/html/transform-table-007.htm
  └   → /css-transforms-1_dev/html/transform-table-007.htm a5c014b20ef1363bea6f24eda28c7efb7c45698a
/css-transforms-1_dev/html/reference/transform-blank-ref.htm fa6407b1acbbfea27e27061e7d1bdeca98e4a728
Testing a5c014b20ef1363bea6f24eda28c7efb7c45698a == fa6407b1acbbfea27e27061e7d1bdeca98e4a728

Ms2ger · 2016-06-15T07:51:13Z

@bors-servo retry #11574

bors-servo · 2016-06-15T07:51:15Z

⚡ Previous build results for android, arm32, arm64, linux-dev, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows are reusable. Rebuilding only linux-rel...

bors-servo · 2016-06-15T08:38:25Z

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows

highfive assigned mbrubeck Jun 10, 2016

highfive added the S-awaiting-review label Jun 10, 2016

mrmiywj mentioned this pull request Jun 10, 2016

form element accessed by index #11657

Closed

4 of 5 tasks complete

jdm assigned Ms2ger and unassigned mbrubeck Jun 10, 2016

Ms2ger reviewed Jun 10, 2016
View changes

tests/wpt/web-platform-tests/html/semantics/forms/the-form-element/form-indexed-element.html Outdated

<form id=form>

<table>

<tr>

<td><input type="radio" name="radio1" id="r1" value=1></td>

This comment has been minimized.

Sign in to view

Ms2ger Jun 10, 2016
Contributor

Why the table?

frewsxcv mentioned this pull request Jun 11, 2016

Implement HTMLFormElement Element getter #11405

Closed

KiChjang reviewed Jun 13, 2016
View changes

KiChjang added S-needs-code-changes and removed S-awaiting-review labels Jun 13, 2016

highfive added S-awaiting-review and removed S-needs-code-changes labels Jun 13, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 14, 2016

highfive added S-awaiting-review and removed S-tests-failed labels Jun 14, 2016

jdm added S-needs-squash and removed S-awaiting-review labels Jun 15, 2016

indexed getter of form elements

Loading status checks…

4bb8843

highfive added the S-awaiting-review label Jun 15, 2016

highfive added S-awaiting-merge and removed S-awaiting-review S-needs-squash labels Jun 15, 2016

highfive added S-tests-failed and removed S-awaiting-merge labels Jun 15, 2016

bors-servo merged commit 4bb8843 into servo:master Jun 15, 2016
2 checks passed

2 checks passed

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

homu Test successful
Details

bors-servo mentioned this pull request Jun 15, 2016

Update web-platform-tests to revision 98b24f230d8d9b04187c0e03e4f33fc205637b15 #11731

Closed

servo / servo

indexed getter of form elements #11697

indexed getter of form elements #11697

mrmiywj commented Jun 10, 2016 •

edited by larsbergstrom

highfive commented Jun 10, 2016

Ms2ger commented Jun 10, 2016

mrmiywj commented Jun 10, 2016

This comment has been minimized.

This comment has been minimized.

highfive commented Jun 10, 2016

mrmiywj commented Jun 10, 2016

KiChjang commented Jun 11, 2016

mrmiywj commented Jun 11, 2016

highfive commented Jun 12, 2016

highfive commented Jun 12, 2016

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

highfive commented Jun 13, 2016

mrmiywj commented Jun 13, 2016

Ms2ger commented Jun 13, 2016

highfive commented Jun 13, 2016

mrmiywj commented Jun 13, 2016

jdm commented Jun 13, 2016

highfive commented Jun 13, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 15, 2016

mrmiywj commented Jun 15, 2016

KiChjang commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

highfive commented Jun 15, 2016

Ms2ger commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

servo / servo

Join GitHub today

indexed getter of form elements #11697

indexed getter of form elements #11697

Conversation

mrmiywj commented Jun 10, 2016 • edited by larsbergstrom

highfive commented Jun 10, 2016

Ms2ger commented Jun 10, 2016

mrmiywj commented Jun 10, 2016

This comment has been minimized.

Ms2ger Jun 10, 2016

This comment has been minimized.

Ms2ger Jun 10, 2016

highfive commented Jun 10, 2016

mrmiywj commented Jun 10, 2016

KiChjang commented Jun 11, 2016

mrmiywj commented Jun 11, 2016

highfive commented Jun 12, 2016

highfive commented Jun 12, 2016

This comment has been minimized.

KiChjang Jun 13, 2016

This comment has been minimized.

mrmiywj Jun 13, 2016

This comment has been minimized.

KiChjang Jun 13, 2016 • edited

highfive commented Jun 13, 2016

mrmiywj commented Jun 13, 2016

Ms2ger commented Jun 13, 2016

highfive commented Jun 13, 2016

mrmiywj commented Jun 13, 2016

jdm commented Jun 13, 2016

highfive commented Jun 13, 2016

bors-servo commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 14, 2016

highfive commented Jun 14, 2016

jdm commented Jun 15, 2016

mrmiywj commented Jun 15, 2016

KiChjang commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

highfive commented Jun 15, 2016

Ms2ger commented Jun 15, 2016

bors-servo commented Jun 15, 2016

bors-servo commented Jun 15, 2016

mrmiywj commented Jun 10, 2016 •

edited by larsbergstrom

KiChjang Jun 13, 2016 •

edited