Skip to content

/ servo

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Emit change event when files are selected #12196

Merged
merged 1 commit into from Jul 4, 2016
Merged

Emit change event when files are selected #12196

merged 1 commit into from Jul 4, 2016

Conversation

@izgzhen
Copy link
Contributor

izgzhen commented Jul 3, 2016 •
edited

r? @Manishearth

As specified in step 6 of activation behaviour.

Part of #11131.

This change is Reviewable
@highfive
Copy link

highfive commented Jul 3, 2016

Heads up! This PR modifies the following files:

  • @KiChjang: components/script/dom/htmlinputelement.rs
@highfive
Copy link

highfive commented Jul 3, 2016

warning Warning warning

  • These commits modify script code, but no tests are modified. Please consider adding a test!
@nox
Copy link
Member

nox commented Jul 3, 2016

Where is the first event firing done? The spec says:

fire a simple event that bubbles named input at the input element, and finally fire a simple event that bubbles named change at the input element.

This PR just does the second part, right?
@izgzhen
Copy link
Contributor Author

izgzhen commented Jul 3, 2016

@nox Ooops. Thanks a lot -- I was testing with onchange thus somehow overlooks this point .. My bad
@izgzhen izgzhen force-pushed the izgzhen:emit-change-file-select branch from 0552398 to ef17c98 Jul 3, 2016
@Manishearth
Copy link
Member

Manishearth commented Jul 3, 2016 

./components/script/dom/htmlinputelement.rs:1153: link to WHATWG may break in the future, use this format instead: https://html.spec.whatwg.org/multipage/#file-upload-state-

tidy fix, r=me
@nox
Copy link
Member

nox commented Jul 3, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2016

⌛ Trying commit ef17c98 with merge 05c2625...
bors-servo added a commit that referenced this pull request Jul 3, 2016
@bors-servo
Auto merge of #12196 - izgzhen:emit-change-file-select, r=<try>
05c2625 
Emit change event when files are selected

<!-- Please describe your changes on the following line: -->

r? @Manishearth

As specified in step 6 of [activation behaviour](https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-(type=file)).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12196)
<!-- Reviewable:end -->
@izgzhen
Emit change event when files are selected
Loading status checks…
411b1b5
@izgzhen izgzhen force-pushed the izgzhen:emit-change-file-select branch from ef17c98 to 411b1b5 Jul 3, 2016
@izgzhen
Copy link
Contributor Author

izgzhen commented Jul 3, 2016

Fixed
@Manishearth
Copy link
Member

Manishearth commented Jul 3, 2016

Alright. I suspect some WPT tests will pass now, so let's wait for the try build, update expectations, and then r=me.
@izgzhen
Copy link
Contributor Author

izgzhen commented Jul 3, 2016

@bors-servo try
@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2016

⌛ Trying commit 411b1b5 with merge 3a2239e...
bors-servo added a commit that referenced this pull request Jul 3, 2016
@bors-servo
Auto merge of #12196 - izgzhen:emit-change-file-select, r=<try>
3a2239e 
Emit change event when files are selected

<!-- Please describe your changes on the following line: -->

r? @Manishearth

As specified in step 6 of [activation behaviour](https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-(type=file)).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12196)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@Manishearth
Copy link
Member

Manishearth commented Jul 3, 2016

@bors-servo r+
@bors-servo
Copy link
Contributor

bors-servo commented Jul 3, 2016

📌 Commit 411b1b5 has been approved by Manishearth
@izgzhen
Copy link
Contributor Author

izgzhen commented Jul 3, 2016

Why homu is not merging this?
@KiChjang
Copy link
Member

KiChjang commented Jul 4, 2016

@izgzhen This still hasn't been tested yet, so homu is not merging it.
@izgzhen
Copy link
Contributor Author

izgzhen commented Jul 4, 2016 •
edited

@KiChjang http://build.servo.org/builders/mac-rel-css/builds/1858?

Ok beside this mac-rel-css, the status on queue shows that it is "success"
@KiChjang
Copy link
Member

KiChjang commented Jul 4, 2016

Looks like there were issues in the build system.

@bors-servo retry r=Manishearth clean
@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2016

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #12200
@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2016

📌 Commit 411b1b5 has been approved by Manishearth
@Manishearth
Copy link
Member

Manishearth commented Jul 4, 2016

@bors-servo try-
@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2016

⌛ Testing commit 411b1b5 with merge 110fd68...
bors-servo added a commit that referenced this pull request Jul 4, 2016
@bors-servo
Auto merge of #12196 - izgzhen:emit-change-file-select, r=Manishearth
Loading status checks…
110fd68 
Emit change event when files are selected

<!-- Please describe your changes on the following line: -->

r? @Manishearth

As specified in step 6 of [activation behaviour](https://html.spec.whatwg.org/multipage/forms.html#file-upload-state-(type=file)).

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy` does not report any errors

<!-- Either: -->
- [ ] There are tests for these changes OR
- [ ] These changes do not require tests because _____

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/12196)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Jul 4, 2016

☀️ Test successful - android, arm32, arm64, linux-dev, linux-rel, mac-dev-unit, mac-rel-css, mac-rel-wpt, windows
@bors-servo bors-servo merged commit 411b1b5 into servo:master Jul 4, 2016
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
homu Test successful
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@Manishearth Manishearth

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants
@izgzhen @highfive @nox @Manishearth @bors-servo @KiChjang
You can’t perform that action at this time.