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

[HackerNews] Fix tag containing comment #4226

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

Conversation

rakoo
Copy link
Contributor

@rakoo rakoo commented Aug 21, 2024

Caution: this is untested because I have trouble running the docker command, but it's minor

Copy link

Pull request artifacts

Bridge Context Status
HackerNewsUserThreads 1 untitled (current) Bridge returned error 0! (19956)
Type: ErrorException
Message: Attempt to read property "innertext" on null
HackerNewsUserThreads 1 untitled (pr) Bridge returned error 0! (19956)
Type: ErrorException
Message: Attempt to read property "innertext" on null

last change: Wednesday 2024-08-21 18:16:34

@dvikan
Copy link
Contributor

dvikan commented Aug 21, 2024

it already broken from before.

your suggestion suffers the same fate:

Details
Type: ErrorException
Code: 0
Message: Attempt to read property "innertext" on null
File: bridges/HackerNewsUserThreadsBridge.php
Line: 43

@rakoo
Copy link
Contributor Author

rakoo commented Sep 19, 2024

So, I don't know how to fix this; it now "works on my machine" (but I know how bad this sentence is)

Any help on what I can do to have a look at that ?

@Bockiii
Copy link
Contributor

Bockiii commented Oct 18, 2024

I dug into this a bit and it looks like the .innertext in line 43 is correct in general.

Is it possible that the innertext might be empty for some comments? If the comment is an image for example? I'm not sure of the capabilities of the thread board.

Can you add an error handling if the innertext of the comment is empty, then just set it to "" or something?

if (empty($element->find('span[class*="commtext"]', 0)->innertext))

[HackerNews] Fix tag containing comment
@rakoo
Copy link
Contributor Author

rakoo commented Oct 19, 2024

No, HackerNews is a text-only message board. Moreove the error seemed to be related to the containing div being null. I put a null-check on it.

The build is broken because of #4310, fixed in #4311

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