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.

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

Update Graby.php #352

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Update Graby.php #352

wants to merge 1 commit into from

Conversation

Strubbl
Copy link

@Strubbl Strubbl commented Sep 27, 2024

keep empty img tags to avoid missing images, e.g. wallabag/wallabag#7700

keep empty img tags to avoid missing images, e.g. wallabag/wallabag#7700
@coveralls
Copy link

Coverage Status

coverage: 94.897%. remained the same
when pulling cb190fc on Strubbl:patch-1
into 1281bf3 on j0k3r:master.

@jtojnar
Copy link
Collaborator

jtojnar commented Sep 27, 2024

Thanks for the contribution.

Would you be able to include a test like the following?

graby/tests/GrabyTest.php

Lines 1000 to 1008 in 1281bf3

public function testEmptyNodesRemoved(): void
{
$graby = $this->getGrabyWithMock('/fixtures/content/framablog.html');
$res = $graby->fetchContent('https://framablog.org/2017/12/02/avancer-ensemble-vers-la-contribution/');
// The initial treatment was encapsulating the content into the empty node
// So we don't want to see that again
$this->assertStringNotContainsString('<figure><p>Après un <em>icebreaker</em>', $res->getHtml());
}


Aside, I find it weird that #141 was made against Graby rather than Readability in the first place but whatever 🤷‍♀️

@Strubbl
Copy link
Author

Strubbl commented Oct 10, 2024

Sorry, i am not sure how to write and run a proper test here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants