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

Fix panic from update_href in HTMLAnchorElement #11264

Merged
merged 1 commit into from May 19, 2016
Merged

Fix panic from update_href in HTMLAnchorElement #11264

merged 1 commit into from May 19, 2016

Conversation

@KiChjang
Copy link
Member

KiChjang commented May 19, 2016 •
edited by larsbergstrom

  • ./mach build -d does not report any errors
  • ./mach test-tidy --faster does not report any errors
  • These changes fix #11261 (github issue number if applicable).
  • These changes do not require tests because unsure how this would be tested.

This change is Reviewable
@KiChjang
Fix panic from update_href in HTMLAnchorElement
Loading status checks…
deadfa6
@highfive
Copy link

highfive commented May 19, 2016

Heads up! This PR modifies the following files:

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

highfive commented May 19, 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 May 19, 2016

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

bors-servo commented May 19, 2016

⌛ Trying commit deadfa6 with merge edeff8c...
bors-servo added a commit that referenced this pull request May 19, 2016
@bors-servo
Auto merge of #11264 - KiChjang:fix-anchor-panic, r=<try>
edeff8c 
Fix panic from update_href in HTMLAnchorElement

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #11261 (github issue number if applicable).
- [x] These changes do not require tests because unsure how this would be tested.

<!-- 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/11264)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

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

metajack commented May 19, 2016

@bors-servo r+

This is a nice cleanup. The pattern is almost the same everywhere such that it seems like it could be abstracted into a function which took a closure for a test and a closure for the success branch. But if you agree that is a good idea, it's doable in a followup.

Previously, bors-servo wrote…

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

Reviewed 1 of 1 files at r1.
Review status: all files reviewed at latest revision, all discussions resolved.

Comments from Reviewable
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

📌 Commit deadfa6 has been approved by metajack
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

⌛ Testing commit deadfa6 with merge 8acc742...
bors-servo added a commit that referenced this pull request May 19, 2016
@bors-servo
Auto merge of #11264 - KiChjang:fix-anchor-panic, r=metajack
8acc742 
Fix panic from update_href in HTMLAnchorElement

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #11261 (github issue number if applicable).
- [x] These changes do not require tests because unsure how this would be tested.

<!-- 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/11264)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

💔 Test failed - linux-rel
@nox
Copy link
Member

nox commented May 19, 2016

@bors-servo r=metajack
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

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

  • This pull request previously failed. You should add more commits to fix the bug, or use retry to trigger a build again.
  • There's another pull request that is currently being tested, blocking this pull request: #11263
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

📌 Commit deadfa6 has been approved by metajack
@nox
Copy link
Member

nox commented May 19, 2016

@bors-servo retry
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 2016

⌛ Testing commit deadfa6 with merge 5bf2849...
bors-servo added a commit that referenced this pull request May 19, 2016
@bors-servo
Auto merge of #11264 - KiChjang:fix-anchor-panic, r=metajack
Loading status checks…
5bf2849 
Fix panic from update_href in HTMLAnchorElement

- [x] `./mach build -d` does not report any errors
- [x] `./mach test-tidy --faster` does not report any errors
- [x] These changes fix #11261 (github issue number if applicable).
- [x] These changes do not require tests because unsure how this would be tested.

<!-- 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/11264)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented May 19, 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 deadfa6 into servo:master May 19, 2016
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
@jdm jdm mentioned this pull request May 19, 2016
Open
@KiChjang KiChjang deleted the KiChjang:fix-anchor-panic branch May 19, 2016
@KiChjang KiChjang mentioned this pull request May 19, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@metajack metajack

Labels
None yet
Projects
None yet
Milestone
No milestone
Linked issues

Successfully merging this pull request may close these issues.

Servo crash in script::dom::htmlanchorelement::HTMLAnchorElement::update_href
5 participants
@KiChjang @highfive @nox @bors-servo @metajack
You can’t perform that action at this time.