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

Jump to bottom

Do not use u8 in HTMLIFrameElement::sandbox #11599

Closed
nox opened this issue Jun 4, 2016 · 14 comments
Closed

Do not use u8 in HTMLIFrameElement::sandbox #11599

nox opened this issue Jun 4, 2016 · 14 comments
Labels
A-content/dom C-assigned E-easy I-enhancement

Comments

@nox
Copy link
Member

@nox nox commented Jun 4, 2016

The field sandbox of HTMLIFrameElement should be changed to Cell<Option<SandboxAllowance>>.
@highfive
Copy link

@highfive highfive commented Jun 4, 2016

Please make a comment here if you intend to work on this issue. Thank you!
@nox nox changed the title Make HTMLIFrameElement::sandbox a SandboxAllowance Do not use u8 in HTMLIFrameElement::sandbox Jun 4, 2016
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 4, 2016 •
edited

@nox I want to fix this one.
@nox
Copy link
Member Author

@nox nox commented Jun 4, 2016

Ok.
@jdm jdm added the C-assigned label Jun 4, 2016
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 9, 2016

@nox I am going to be a little busy this month and the beginning of the next, I will do my best to get this issue solved between all the other things I have going on. Just to confirm, the issue is at this file (at line 66): https://github.com/servo/servo/blob/master/components/script/dom/htmliframeelement.rs
@nox
Copy link
Member Author

@nox nox commented Jun 9, 2016

Yes exactly!
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 17, 2016

@nox Please see this fix: #11765

It is a combination of your issue and @asajeffrey
@KiChjang KiChjang mentioned this issue Jun 17, 2016
Closed
@nox
Copy link
Member Author

@nox nox commented Jun 17, 2016

Please do 2 separate PRs.
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 17, 2016

@nox Ok, I had trouble getting GitHub Desktop to do that, it just wanted to add to the last PR...
@nox
Copy link
Member Author

@nox nox commented Jun 17, 2016

Just make a new branch with only the commit for that fix and make a PR out of it.
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 17, 2016

@nox Ok perfect, will do momentarily
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 17, 2016

@nox I just re cloned the repo as there were many errors and two PRs to mess with, I am just compiling to make sure it is running error free on my end, then I am going to PR the fix for your issue.
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 17, 2016

@nox I am very sorry for saying I was going to remake a PR "momentarily" and now here we are 10 hours later. There were a few unexpected events in my day. It is my goal to have it done for this weekend, hopefully tonight.
@Coder206
Copy link
Contributor

@Coder206 Coder206 commented Jun 21, 2016 •
edited

@nox You mean the sandbox_allowance field (https://github.com/servo/servo/blob/master/components/script/dom/htmliframeelement.rs#L70)?
@jdm
Copy link
Member

@jdm jdm commented Jun 21, 2016

Yes.
@Coder206 Coder206 mentioned this issue Jul 1, 2016
Merged
4 of 4 tasks complete
bors-servo added a commit that referenced this issue Jul 2, 2016
@bors-servo
Auto merge of #12113 - Coder206:u8, r=mbrubeck,emilio
Loading status checks…
40909ed 
Removing u8 from HTMLIframeElement.rs file

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

---
<!-- 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
- [x] These changes fix #11599 (github issue number if applicable).

<!-- Either: -->
- [x] These changes do not require tests because @nox did not request any so I am not sure which to do.

<!-- 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/12113)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-content/dom C-assigned E-easy I-enhancement
Projects
None yet
Milestone
No milestone
Linked pull requests

Successfully merging a pull request may close this issue.

Removing u8 from HTMLIframeElement.rs file
4 participants
@jdm @nox @highfive @Coder206
You can’t perform that action at this time.