Skip to content

Wrap non-seekable stream in XamlToRtfWriter #995

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

Merged
merged 4 commits into from
Jun 24, 2019

Conversation

stevenbrix
Copy link
Contributor

@stevenbrix stevenbrix commented Jun 18, 2019

When we started using System.IO.Packaging that came in corefx as part of this commit 71c9337d, the behavior of the streams that were returned from PackagePart.GetStream are now non-seekable. The behavior change of this issue is described in #585, with links to the corresponding corefx issue that discusses the behavior change. The workaround for the behavior change is to wrap the stream in a MemoryStream to allow seek-ability.

Due to the refactor, the zip implementation inside of WindowsBase is no longer needed. I've removed it as part of this change since this should have been removed with the original refactor, but was missed.

The PackUriHelper.GetPartUri method was also missing in CoreFx. This was added with dotnet/corefx#38699. I've updated the code to remove more of our custom duplicated logic in favor for what is in corefx. No code that is in our internal branch depends on these APIs, they already use the PackUriHelper APIs that come from System.IO.Packaging.

Fixes #858
Fixes #597

@ghost ghost requested review from vatsan-madhavan, rladuca and ryalanms June 18, 2019 06:08
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 18, 2019
Copy link
Member

@vatsan-madhavan vatsan-madhavan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm.

the previously failing tests pass now, do they?

@@ -331,35 +328,6 @@
<Compile Include="System\Windows\Markup\XmlPartReader.cs" />
</ItemGroup>
<!-- ZIP sources -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove comment

@stevenbrix stevenbrix force-pushed the dev/stevenbrix/xamlToRtfWriterFix branch from 05cd5b6 to 2db4fec Compare June 22, 2019 14:54
@stevenbrix
Copy link
Contributor Author

lgtm.

the previously failing tests pass now, do they?

@vatsan-madhavan, yeah the tests pass. Some of theme are no longer needed (like DrtZipIO) and quite a few failures were also just due to test code needing to be updated. Most of the tests use reflection to discover and invoke methods and that is the biggest reason they've started failing (rather than breaking at compile time). They were either trying to reflect and invoke internal methods that were removed, or the parameters were slightly different (removal of the bool streaming parameter from Package.Open). Most tests don't even rely on the streaming functionality that existed. This is at least true for the DRTs, I'm not sure what subset of feature tests that might be broken, but fixing those should be fairly straightforward as well.

@stevenbrix stevenbrix force-pushed the dev/stevenbrix/xamlToRtfWriterFix branch from a4aa449 to f1d14ae Compare June 24, 2019 16:45
@vatsan-madhavan vatsan-madhavan added this to the Preview milestone Jun 24, 2019
@stevenbrix stevenbrix merged commit 08128b9 into master Jun 24, 2019
@vatsan-madhavan vatsan-madhavan deleted the dev/stevenbrix/xamlToRtfWriterFix branch August 23, 2019 20:11
vatsan-madhavan pushed a commit that referenced this pull request Sep 6, 2019
* removing of zip

* fixing XamlToRtfWriter

* using PackUriHelper.GetPartUri that's in CoreFx

* removing comment
@ghost ghost locked as resolved and limited conversation to collaborators Apr 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
PR metadata: Label to tag PRs, to facilitate with triage
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[netcore3] XamlToRtfWriter.WriteShapeImage fails on unseekable streams Printing fails in WPF
3 participants