Skip to content

Fix image missing when using Base64 content #277

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 2 commits into from
May 14, 2020

Conversation

christiansandberg
Copy link
Contributor

When adding a Base64 encoded image to a non-self contained report, only an empty link is created but no image.

@christiansandberg
Copy link
Contributor Author

@BeyondEvil could you have a look at this? Should be quite trivial.

@BeyondEvil
Copy link
Contributor

Thanks! I'm planning on putting in some time during the weekend to look at all open issues and PRs.

@ssbarnea
Copy link
Member

Please rebase. Thanks.

@christiansandberg christiansandberg force-pushed the fix-missing-image branch 2 times, most recently from 3af781d to 3426bea Compare March 21, 2020 18:39
@christiansandberg
Copy link
Contributor Author

@ssbarnea I’ve rebased my branch.

@dhalperi
Copy link
Contributor

Darn it, I am the third person to create this pr [#298, after #287] with mildly different tests. @BeyondEvil - any way to review/merge this?

Related to #265. [now it's cross-linked to the issue and maybe a fourth person won't write it :).]

@dhalperi
Copy link
Contributor

cc: @ssbarnea

@BeyondEvil
Copy link
Contributor

Sorry for not getting to this sooner @christiansandberg

tl;dr had spine surgery which led to complications.

Anyway, as soon as I'm satisfied with understanding the issue this is fixing (just want to see it with my own eyes) I'm going to merge this.

@dhalperi Sorry you had to do all that work, but you should've checked existing issues and PRs amirite 😉 What I would like for you to do, once this is merged, is to rebase. What will be left is basically that extra test (which is awesome!) but I also want you to update CHANGES.rst and give yourself and Christian credit. 😄For version you can just bump the patch number and for date put unreleased.

All good?

@dhalperi
Copy link
Contributor

@BeyondEvil - your health takes top priority, I hope you're doing okay now. Glad to see you are able to work again. Everyone's having a different thing happening in this crazy time...

I am happy to support in whatever mode is best! Let me know. (Also, if there's a gitter or slack channel that. might better facilitate realtime communication, I can go there)

@BeyondEvil
Copy link
Contributor

BeyondEvil commented May 13, 2020

I'm jimboslice in the official SeleniumHQ slack @dhalperi :)

Copy link
Contributor

@BeyondEvil BeyondEvil left a comment

Choose a reason for hiding this comment

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

Excellent work!

Again, sorry it took a while.

@BeyondEvil BeyondEvil merged commit 484c0ce into pytest-dev:master May 14, 2020
@dhalperi
Copy link
Contributor

dhalperi commented Jul 6, 2020

@BeyondEvil - the official selenium slack HQ link (from here) has expired - maybe someone there should refresh it. The question I came in to ask - will it be possible to make a new release with this fix sometime soon?

@ssbarnea ssbarnea added the bug This issue/PR relates to a bug. label Aug 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue/PR relates to a bug.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants