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 Implement HTMLFormElement Element getter #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.

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.

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 :)

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...

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 There is new code that needs to be reviewed. label Jun 10, 2016

mrmiywj mentioned this pull request Jun 10, 2016

form element accessed by index #11657

Closed

5 tasks

jdm assigned Ms2ger and unassigned mbrubeck Jun 10, 2016

Ms2ger reviewed Jun 10, 2016
View reviewed changes

frewsxcv mentioned this pull request Jun 11, 2016

Implement HTMLFormElement Element getter #11405

Closed

KiChjang reviewed Jun 13, 2016
View reviewed changes

KiChjang added S-needs-code-changes Changes have not yet been made that were requested by a reviewer. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 13, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-needs-code-changes Changes have not yet been made that were requested by a reviewer. labels Jun 13, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jun 14, 2016

highfive added S-awaiting-review There is new code that needs to be reviewed. and removed S-tests-failed The changes caused existing tests to fail. labels Jun 14, 2016

jdm added S-needs-squash Some (or all) of the commits in the PR should be combined. and removed S-awaiting-review There is new code that needs to be reviewed. labels Jun 15, 2016

indexed getter of form elements

4bb8843

highfive added the S-awaiting-review There is new code that needs to be reviewed. label Jun 15, 2016

highfive added S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. and removed S-awaiting-review There is new code that needs to be reviewed. S-needs-squash Some (or all) of the commits in the PR should be combined. labels Jun 15, 2016

highfive added S-tests-failed The changes caused existing tests to fail. and removed S-awaiting-merge The PR is in the process of compiling and running tests on the automated CI. labels Jun 15, 2016

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

bors-servo mentioned this pull request Jun 15, 2016

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

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

KiChjang Jun 13, 2016

mrmiywj Jun 13, 2016

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

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

Ms2ger Jun 10, 2016

Choose a reason for hiding this comment

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

KiChjang Jun 13, 2016

Choose a reason for hiding this comment

mrmiywj Jun 13, 2016

Choose a reason for hiding this comment

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