Skip to content
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.

Sign up for GitHub

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

Use a streaming base64 encoder in canvas.toDataURL #22147

Closed
wants to merge 1 commit into from
Closed

Use a streaming base64 encoder in canvas.toDataURL #22147

wants to merge 1 commit into from

Conversation

nox
Copy link
Contributor

@nox nox commented Nov 8, 2018 •
edited by SimonSapin

This change is Reviewable

@highfive
Copy link

highfive commented Nov 8, 2018

Heads up! This PR modifies the following files:

  • @asajeffrey: components/script/Cargo.toml, components/script/dom/htmlcanvaselement.rs, components/webdriver_server/Cargo.toml
  • @jgraham: components/webdriver_server/Cargo.toml
  • @edunham: servo-tidy.toml
  • @KiChjang: components/net/Cargo.toml, components/script/Cargo.toml, components/script/dom/htmlcanvaselement.rs

@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 8, 2018
@highfive
Copy link

highfive commented Nov 8, 2018

warning Warning warning

  • These commits modify net and script code, but no tests are modified. Please consider adding a test!

nox added a commit that referenced this pull request Nov 8, 2018
@nox
Use a streaming base64 encoder in canvas.toDataURL (fixes #22147)
ba9f2f4
@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #22151) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Nov 9, 2018
@jdm jdm removed the S-awaiting-review There is new code that needs to be reviewed. label Nov 9, 2018
nox added a commit that referenced this pull request Nov 9, 2018
@nox
Use a streaming base64 encoder in canvas.toDataURL (fixes #22147)
de54aef
@highfive highfive added the S-awaiting-review There is new code that needs to be reviewed. label Nov 9, 2018
@nox
Copy link
Contributor Author

nox commented Nov 9, 2018

@bors-servo r=jdm

@bors-servo
Copy link
Contributor

📌 Commit de54aef has been approved by jdm

@highfive 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-rebase There are merge conflict errors. labels Nov 9, 2018
@bors-servo
Copy link
Contributor

⌛ Testing commit de54aef with merge c3956f8...

bors-servo pushed a commit that referenced this pull request Nov 9, 2018
Auto merge of #22147 - servo:base64, r=jdm
c3956f8 
Use a streaming base64 encoder in canvas.toDataURL

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

💔 Test failed - linux-rel-css

@highfive 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 Nov 9, 2018
@nox
Copy link
Contributor Author

nox commented Nov 9, 2018

Seems like I broke something or the encoder is broken.

@jdm
Copy link
Member

jdm commented Nov 9, 2018

@bors-servo r-

@nox nox mentioned this pull request Nov 13, 2018
Merged
@nox
Copy link
Contributor Author

nox commented Nov 13, 2018

image-rs/image-png#94

@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #22209) made this pull request unmergeable. Please resolve the merge conflicts.

@highfive highfive added the S-needs-rebase There are merge conflict errors. label Nov 18, 2018
@highfive 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 Nov 21, 2018
@bors-servo
Copy link
Contributor

☔ The latest upstream changes (presumably #22225) made this pull request unmergeable. Please resolve the merge conflicts.

@jdm jdm added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. and removed S-awaiting-review There is new code that needs to be reviewed. labels Nov 28, 2018
@atouchet
Copy link
Contributor

atouchet commented Nov 29, 2018 •
edited

png 0.13.2 is now out.

Edit: I guess image-rs/image#837 still needs to be merged as well.

@atouchet atouchet added S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. and removed S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. labels Nov 29, 2018
@nox
Copy link
Contributor Author

nox commented Jan 23, 2019

I will reopen this when the 24,787 blockers are fixed.

@nox nox closed this Jan 23, 2019
@nox nox deleted the base64 branch January 23, 2019 16:14
@atouchet
Copy link
Contributor

image and png were updated in #22973. Is there anything else blocking this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jdm jdm jdm approved these changes

Assignees

@jdm jdm

Labels
S-blocked-on-external Something, somewhere else, needs to happen before this PR can be merged. S-needs-rebase There are merge conflict errors.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@nox @highfive @bors-servo @jdm @atouchet